bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: New module argp-version-etc


From: Eric Blake
Subject: Re: New module argp-version-etc
Date: Wed, 24 Jun 2009 15:17:03 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

Sergey Poznyakoff <gray <at> gnu.org.ua> writes:

> The new module argp-version-etc (patch 2) is designed to
> facilitate the use of argp and version-etc modules together.
> This will ensure uniform version output between several
> programs within the same project, and will be useful for
> such projects as, for example, GNU Inetutils. 

Sounds nice to me.

> 
> -/* Like version_etc, below, but with the NULL-terminated author list
> -   provided via a variable of type va_list.  */
>  void
> -version_etc_va (FILE *stream,
> -             const char *command_name, const char *package,
> -             const char *version, va_list authors)
> +version_etc_arn (FILE *stream,
> +              const char *command_name, const char *package,
> +              const char *version, size_t n_authors, const char *authors[])

Why'd you drop the comments describing what the method does?  Every method 
should be documented.  I'd like to see the arguments reversed:

const char *authors[], size_t n_authors

for consistency with other functions that take both an array and a length (for 
example, getcwd).

> +void
> +version_etc_va (FILE *stream,
> +             const char *command_name, const char *package,
> +             const char *version, va_list authors)
> +{
> +  size_t n_authors;
> +  const char *authtab[10];
> +  
> +  for (n_authors = 0;
> +       n_authors < 10
> +      && (authtab[n_authors++] = va_arg (authors, const char *)) != NULL;
> +       n_authors++)
> +    ;

missing va_end (but then again, the original version_etc_va was also missing a 
va_end(tmp_authors)).  Now that you no longer depend on va_copy, should the 
module specification be simplified to drop dependence on stdarg?

> +++ b/tests/test-ave-2.sh
...
> +cat > $TMP <<EOT
> +test-ave (dummy) 0
> +Copyright (C) 2009 Free Software Foundation, Inc.

Are we going to have to update this test every year?  It would be nice to 
compute the year that should be present, rather than hard-coding it.

-- 
Eric Blake






reply via email to

[Prev in Thread] Current Thread [Next in Thread]