[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 21 Jun 2014 12:18:14 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH v3] Implement fmtmsg
On Sat, Jun 21, 2014 at 03:56:38PM +0000, Isaac Dunham wrote:
> > This is the wrong check for consoldfd being valid; 0 is a valid fd,
> > whereas consolefd==-1 passes the test.
> >
> > So I think consolefd should be initialized to -1 not 0 and the check
> > here should be >=0. Or we could just reorder the severity lines above
> > the opening of the console so that the opening and use of consolefd
> > aren't separated like this.
>
> As far as I can tell:
> -severity really should go above this (grouping related subjects)
OK we agree on this.
> -initializing to -1 will expose a bit of brokenness in here:
> RET=MM_NOCON;
> would be executed whenever we did not successfully open the console,
> whether or not we tried to open it.
> It would be better to
> * leave the initialization alone, or move the error block into the same
> if() {} block as the call to open().
The latter is better. Using 0 to mean "none" for a file descriptor
variable is bad practice in general.
BTW while making these changes I noticed that you never close
consolefd. I added that in the same place. Also made a few other
improvements. See if the attached file looks better.
Rich
View attachment "fmtmsg.c" of type "text/plain" (2474 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
