pfctl: Fix function name in errx(3) message

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

pfctl: Fix function name in errx(3) message

Klemens Nanni
Index: pfctl.c
===================================================================
RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
retrieving revision 1.338
diff -u -p -r1.338 pfctl.c
--- pfctl.c     26 Jan 2017 08:24:34 -0000      1.338
+++ pfctl.c     25 Mar 2017 11:37:01 -0000
@@ -753,7 +753,7 @@ pfctl_show_rules(int dev, char *path, in
        memset(&pr, 0, sizeof(pr));
        if (anchorname[0] == '/') {
                if ((npath = calloc(1, PATH_MAX)) == NULL)
-                       errx(1, "pfctl_rules: calloc");
+                       errx(1, "pfctl_show_rules: calloc");
                strlcpy(npath, anchorname, PATH_MAX);
        } else {
                if (path[0])

Reply | Threaded
Open this post in threaded view
|

Re: pfctl: Fix function name in errx(3) message

Sebastian Benoit-3
Klemens Nanni([hidden email]) on 2017.03.25 12:39:48 +0100:

> Index: pfctl.c
> ===================================================================
> RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
> retrieving revision 1.338
> diff -u -p -r1.338 pfctl.c
> --- pfctl.c     26 Jan 2017 08:24:34 -0000      1.338
> +++ pfctl.c     25 Mar 2017 11:37:01 -0000
> @@ -753,7 +753,7 @@ pfctl_show_rules(int dev, char *path, in
>        memset(&pr, 0, sizeof(pr));
>        if (anchorname[0] == '/') {
>                if ((npath = calloc(1, PATH_MAX)) == NULL)
> -                       errx(1, "pfctl_rules: calloc");
> +                       errx(1, "pfctl_show_rules: calloc");
>                strlcpy(npath, anchorname, PATH_MAX);
>        } else {
>                if (path[0])
>

i commited a err(1, "calloc") instead.
thanks!

Reply | Threaded
Open this post in threaded view
|

Re: pfctl: Fix function name in errx(3) message

Klemens Nanni
On Mon, Mar 27, 2017 at 07:39:09PM +0200, Sebastian Benoit wrote:
>i commited a err(1, "calloc") instead.
What about the other calls to calloc(3); shouldn't we keep their
respective error messages consistent?

Reply | Threaded
Open this post in threaded view
|

Re: pfctl: Fix function name in errx(3) message

Gleydson Soares-3
> What about the other calls to calloc(3); shouldn't we keep their
> respective error messages consistent?

seems that several err()/errx() calls in pfctl code are hard-coding
the function name...

instead of hard code, maybe those calls should be changed to:
errx(1, "%s: anystring", __func__)

Reply | Threaded
Open this post in threaded view
|

Re: pfctl: Fix function name in errx(3) message

Ingo Schwarze
Hi,

Gleydson Soares wrote on Mon, Mar 27, 2017 at 04:43:28PM -0300:

> instead of hard code, maybe those calls should be changed to:
> errx(1, "%s: anystring", __func__)

the usual OpenBSD idiom is

        if ((p = malloc(size)) == NULL)
                err(1, NULL);

see EXAMPLES in malloc(3), simply because the information about
whether it was malloc or calloc or whatever that failed is useless,
and the name of the function where it failed is almost never useful
either.  It's just a matter of chance when exactly memory is
exhausted.  So KISS is the usual approach in this respect.

But it's certainly not a big deal either way.

Yours,
  Ingo

Reply | Threaded
Open this post in threaded view
|

Re: pfctl: Fix function name in errx(3) message

Sebastian Benoit-3
In reply to this post by Gleydson Soares-3
Gleydson Soares([hidden email]) on 2017.03.27 16:43:28 -0300:
> > What about the other calls to calloc(3); shouldn't we keep their
> > respective error messages consistent?
>
> seems that several err()/errx() calls in pfctl code are hard-coding
> the function name...
>
> instead of hard code, maybe those calls should be changed to:
> errx(1, "%s: anystring", __func__)

In this case i removed the function name, because this calloc will only fail
on a bad day and then it does not really matter which calloc is the one oom.

Printing the function name is useful in debug messages or in unusual failure
cases where you need to differentiate between cases.

So yeah, send a diff converting them to __func__, but its fine to remove the
function name in simple cases like malloc/calloc too.