Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 21 Jun 2014 12:18:14 -0400
From: Rich Felker <>
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:
> 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.


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.