Review of the last few applied patches...

[Rich Felker]

On Fri, May 19, 2006 at 01:28:05PM -0400, Rob Landley wrote:
> 15133:
> Since /dev/null isn't going to move, this is just a size optimization, and
> my question is does building all sources at once get this for us?  If so,
> we don't need the complexity, and should probably remove bb_dev_null.  (It'll
> be 2-3 years before building all sources at once becomes the recommended way
> to do it, but we should start thinking about it now and avoid some cleanups
> we'd just have to revert.)

You overestimate gcc... :)

> 15131: mostly fine, although I believe you don't need USE() around a local
> variable declaration unless the code that uses it is entirely #ifdefed out and
> you want to avoid the warning.  If the code using a variable is dead code
> eliminated, then the variables should be too, without a warning.
> 
> Although again, my understanding of what a sane optimizer would do may not
> match the reality of gcc...

It will have an unused variable warning at the least, and may still
allocate space for the variable on the stack..?

> 15129: fine (I'd have tried to clean up the #ifdef while I was there, but
> that's a separate issue.)

This should be moved to a common header or perhaps made into an
external function since equivalent code appears in other modules..

Also the code in other modules is technically incorrect since it uses
.s6_addr32 to access the ipv6 address as 32bit units rather than
.s6_addr which accesses it as bytes. .s6_addr32 is a common extension
(done using union/#define hacks) on many systems but it's not
portable. The code in touched by 15129 is portable (aside from
assuming the address is aligned but hopefully that's a sane
assumption..).

> 15128: I'd have removed it.

Yes..

> 15124: What is strings.h needed for again?  Does this #include make a warning
> go away, or is some future "remove unnecessary #includes" pass going to just
> undo this again?  There's no rationale as to _why_ this was included.  Maybe
> it was in the email.

All legacy/BSD string functions, including strcasecmp and strncasecmp,
are in strings.h according to posix. If you do not provide prototypes
for them, bad things could happen on 64bit systems where sizeof(void*)
!= sizeof(int). I think glibc also prototypes these in string.h but
it's incorrect to do so.

Rich