Re: [PATCH 1/3 v2] unify xmalloc prototypes & friends

[Roland McGrath <roland at hack dot frob dot com>]

> 2012-12-30  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* catgets/gencat.c: Include programs/xmalloc.h.
[...]
> 	* include/programs/xmalloc.h: New file.

It's normal practice to mention the addition of a new file before
mentioning uses of it.  It's not so much a rule as just a style that
makes reading the entry from the top not be confusing.  (I recommend
always reading your log entry from the top while trying to imagine you
had no idea what it was about to begin with.  If you have to read it
through more than once to not be confused, it's not the best way to
structure the entry.)

> 	(xmalloc): Delete.
> 	(xcalloc): Likewise.
> 	(xrealloc): Likewise.
> 	(xstrdup): Likewise.

This looks for all the world like there were function definitions and
you removed them.  This is among the reasons I always tell people I
hate their use of nounless verbs.  It's slightly confusing when you
don't indicate whether it's a variable, a function, or a macro, but
greatly moreso when you don't distinguish declarations from definitions.

I would have written this whole entry like this:

	* include/programs/xmalloc.h: New file.
	* catgets/gencat.c: Include it.
	(xmalloc, xcalloc, xrealloc, xstrdup): Don't declare them.
	* elf/pldd.c: Likewise.
	* iconv/iconv_charmap.c: Likewise.
	...
> +/* Memory related definitions for program modules.
> +   Copyright (C) 1998-2013 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +   Contributed by Ulrich Drepper <drepper@cygnus.com>, 1998.

No "Contributed" lines in new files, and certainly never add one
naming someone else.  In this case, the declarations really could not
be anything else and so there is no meaningful sense in which you have
copied anything from an old file.  So I don't even think the copyright
notice really needs to mention prior years.

> +#include <features.h>

<sys/cdefs.h> is what's really proper if the intent is to express that
you are using the __attribute_* macros.  (In practical terms, you
don't need any such #include if you're including any other public
header, since they all include those.  But include-what-you-use is
good and <sys/cdefs.h> is the right one for that purpose here.)

> +#include <stdlib.h>

The only reason for an #include here is to get size_t defined.
For that, <stddef.h> is the canonical thing to use.

I trust your discretion to clean up these nits as much as you feel
obliged, and then commit this.


Thanks,
Roland