On 2016-03-07 19:22:36 -0600, Derek Martin wrote:
> On Mon, Mar 07, 2016 at 04:32:42PM -0800, Kevin J. McCarthy wrote:
> > Overall the patch looks fine. There are three small changes I'd like to
> > make (attached as a patch file):
> >
> > 1. Since uname sets errno, change the fputs to perror.
> >
> > 2. safe_strdup() checks for NULL params, so remove the redundant
> > NONULL().
> >
> > 3. buffer is allocated on the stack, and can't be NULL, so remove the
> > NONULL.
> >
> > If they look okay to you, I'll just test a bit more and push this out.
>
> That's all fine, though one concern with perror is the error message
> is occasionally unintelligible or confusing in context.
I think this may be useful for debugging.
> But it should really be impossible for uname() to fail, and the man
> page even indicates there are no defined errors. So it probably
> doesn't matter what you do there, in practice.
Unless there's a bug in the kernel. With one of my scripts, I recently
managed to get the following error:
sh: 1: exec: /home/vlefevre/bin/mymaple: File exists
This is unintelligible and confusing, but at least I could have some
idea that it was not a programming error in my script. This is also
an impossible error with execve(), thus apparently a kernel bug.
For the patch, I also suggest to add
dprint(1, (debugfile, "getdnsdomainname(): %s\n", d));
just before the "ret = 0;" line in getdomain.c (this may be useful
for debugging). I used that to also check that the right domain part
was taken into account.
--
Vincent Lefèvre <[email protected]> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
