[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20140621233211.GI179@brightrain.aerifal.cx>
Date: Sat, 21 Jun 2014 19:32:11 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH v3] Implement fmtmsg
On Sat, Jun 21, 2014 at 05:13:12PM +0000, Isaac Dunham wrote:
> On Sat, Jun 21, 2014 at 12:18:14PM -0400, Rich Felker wrote:
> > 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.
>
> It looks much better.
> At this point, consolefd need not be initialized, and I think that will be
> the last change.
Committed. If you see anything else that looks like it needs changing,
just let me know.
Rich
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.
