Re: [PATCH] lib/rename.c: rpl_rename - Avoid unused-but-set-variable compiler warning
On 06/02/2014 08:45 PM, Ben Walton wrote: > The value is set in se > > On Mon, Jun 2, 2014 at 8:31 PM, Pádraig Brady wrote: >> On 06/02/2014 08:13 PM, Ben Walton wrote: >>> * In the non-Win32 variant of rpl_rename, it is possible that >>> dst_exists may be set but not used. Mark it with the unused >>> attribute to avoid compiler warnings. >>> >>> Signed-off-by: Ben Walton >>> --- >>> lib/rename.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/lib/rename.c b/lib/rename.c >>> index 55130d8..099066d 100644 >>> --- a/lib/rename.c >>> +++ b/lib/rename.c >>> @@ -285,7 +285,7 @@ rpl_rename (char const *src, char const *dst) >>>char *dst_temp = (char *) dst; >>>bool src_slash; >>>bool dst_slash; >>> - bool dst_exists; >>> + bool dst_exists _GL_UNUSED; >>>int ret_val = -1; >>>int rename_errno = ENOTDIR; >>>struct stat src_st; >>> >> >> Have to say that I don't see this from looking at the code, >> since it seems to just return or set dst_exists? > > The value is only referenced if RENAME_DEST_EXISTS_BUG is set. > Otherwise, it's set but never used again. This patch addresses the > case where RENAME_DEST_EXISTS_BUG is unset. Ugh sorry. I had read _unused_ as _uninitialized_. Patch is fine and I've just pushed it. thanks! Pádraig.
Re: [PATCH][BZ #16907] Sync argp.h __attribute__ with gnulib.
On Tue, Jun 03, 2014 at 12:59:11PM +0200, Aurelien Jarno wrote: > On Fri, May 23, 2014 at 07:34:57PM +0200, Ondřej Bílka wrote: > > On Fri, May 23, 2014 at 07:53:11AM -0700, Paul Eggert wrote: > > > Ondřej Bílka wrote: > > > >CCing gnulib. Could this be backported also there or do you have > > > >different solution? > > > > > > We solved this problem in a different way years ago, using something > > > like this: > > > > > > #if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR >= 7) > > > # define _GL_ATTRIBUTE_FORMAT(s) __attribute__ ((__format__ s)) > > > #else > > > # define _GL_ATTRIBUTE_FORMAT(s) > > > #endif > > > > > > and then using _GL_ATTRIBUTE_FORMAT ((__printf__, 2, 3)) later, > > > instead of __attribute__ ((__format__ (__printf__, 2, 3))). > > > Presumably glibc could do something similar. Glibc could use a > > > different name; that's all right, gnulib will just switch to the > > > name that glibc uses. > > > > Ok, here is patch that does that with unchanged name (Does somebody have > > better one?). > > > > As I looked at header differences they are mostly minor, like using > > `foo' instead "foo" in comments so it migth be worthwhile to sync them > > up. > > > > > > * argp/argp-fmtstream.h (_GL_ATTRIBUTE_FORMAT): Define. > > (argp_error, argp_failure): Use _GL_ATTRIBUTE_FORMAT. > > * argp/argp.h (__argp_fmtstream_printf): Likewise. > > Thanks for working on this issue. The patch looks fine to me, and I have > just tested it, it fixes the original issue. Please also note that your > patch doesn't apply cleanly, it seems there are some issue with the > context. > Yes, I created that by taking a diff and removing extra parts so that change man typo. A fixed version is here. diff --git a/argp/argp-fmtstream.h b/argp/argp-fmtstream.h index 1ed2834..fc07d91 100644 --- a/argp/argp-fmtstream.h +++ b/argp/argp-fmtstream.h @@ -29,19 +29,16 @@ #include #include -#ifndef __attribute__ -/* This feature is available in gcc versions 2.5 and later. */ -# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 5) || \ - defined __STRICT_ANSI__ -# define __attribute__(Spec) /* empty */ -# endif -/* The __-protected variants of `format' and `printf' attributes - are accepted by gcc versions 2.6.4 (effectively 2.7) and later. */ -# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 7) || \ - defined __STRICT_ANSI__ -# define __format__ format -# define __printf__ printf -# endif +/* The __attribute__ feature is available in gcc versions 2.5 and later. + The __-protected variants of the attributes 'format' and 'printf' are + accepted by gcc versions 2.6.4 (effectively 2.7) and later. + We enable _GL_ATTRIBUTE_FORMAT only if these are supported too, because + gnulib and libintl do '#define printf __printf__' when they override + the 'printf' function. */ +#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 7) +# define _GL_ATTRIBUTE_FORMAT(spec) __attribute__ ((__format__ spec)) +#else +# define _GL_ATTRIBUTE_FORMAT(spec) /* empty */ #endif #if defined (__GNU_LIBRARY__) && defined (HAVE_LINEWRAP_H) @@ -132,10 +129,10 @@ extern void argp_fmtstream_free (argp_fmtstream_t __fs); extern ssize_t __argp_fmtstream_printf (argp_fmtstream_t __fs, const char *__fmt, ...) - __attribute__ ((__format__ (printf, 2, 3))); + _GL_ATTRIBUTE_FORMAT ((printf, 2, 3)); extern ssize_t argp_fmtstream_printf (argp_fmtstream_t __fs, const char *__fmt, ...) - __attribute__ ((__format__ (printf, 2, 3))); + _GL_ATTRIBUTE_FORMAT ((printf, 2, 3)); extern int __argp_fmtstream_putc (argp_fmtstream_t __fs, int __ch); extern int argp_fmtstream_putc (argp_fmtstream_t __fs, int __ch); diff --git a/argp/argp.h b/argp/argp.h index 0868228..6a1cc1b 100644 --- a/argp/argp.h +++ b/argp/argp.h @@ -35,19 +35,16 @@ # define __NTH(fct) fct __THROW #endif -#ifndef __attribute__ -/* This feature is available in gcc versions 2.5 and later. */ -# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 5) || \ - defined __STRICT_ANSI__ -# define __attribute__(Spec) /* empty */ -# endif -/* The __-protected variants of `format' and `printf' attributes - are accepted by gcc versions 2.6.4 (effectively 2.7) and later. */ -# if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 7) || \ - defined __STRICT_ANSI__ -# define __format__ format -# define __printf__ printf -# endif +/* The __attribute__ feature is available in gcc versions 2.5 and later. + The __-protected variants of the attributes 'format' and 'printf' are + accepted by gcc versions 2.6.4 (effectively 2.7) and later. + We enable _GL_ATTRIBUTE_FORMAT only if these are supported too, because + gnulib and libintl do '#define printf __printf__' when they override + the 'printf' function. */ +#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ >= 7) +# define _GL_ATTRIBUTE_FORMAT(spec) __attribute__ ((__format__ spec)) +#el
Re: [PATCH][BZ #16907] Sync argp.h __attribute__ with gnulib.
On Fri, May 23, 2014 at 07:34:57PM +0200, Ondřej Bílka wrote: > On Fri, May 23, 2014 at 07:53:11AM -0700, Paul Eggert wrote: > > Ondřej Bílka wrote: > > >CCing gnulib. Could this be backported also there or do you have > > >different solution? > > > > We solved this problem in a different way years ago, using something > > like this: > > > > #if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR >= 7) > > # define _GL_ATTRIBUTE_FORMAT(s) __attribute__ ((__format__ s)) > > #else > > # define _GL_ATTRIBUTE_FORMAT(s) > > #endif > > > > and then using _GL_ATTRIBUTE_FORMAT ((__printf__, 2, 3)) later, > > instead of __attribute__ ((__format__ (__printf__, 2, 3))). > > Presumably glibc could do something similar. Glibc could use a > > different name; that's all right, gnulib will just switch to the > > name that glibc uses. > > Ok, here is patch that does that with unchanged name (Does somebody have > better one?). > > As I looked at header differences they are mostly minor, like using > `foo' instead "foo" in comments so it migth be worthwhile to sync them > up. > > > * argp/argp-fmtstream.h (_GL_ATTRIBUTE_FORMAT): Define. > (argp_error, argp_failure): Use _GL_ATTRIBUTE_FORMAT. > * argp/argp.h (__argp_fmtstream_printf): Likewise. Thanks for working on this issue. The patch looks fine to me, and I have just tested it, it fixes the original issue. Please also note that your patch doesn't apply cleanly, it seems there are some issue with the context. > @@ -521,12 +546,12 @@ > extern void argp_failure (const struct argp_state *__restrict __state, > int __status, int __errnum, > const char *__restrict __fmt, ...) > - __attribute__ ((__format__ (__printf__, 4, 5))); > + _GL_ATTRIBUTE_FORMAT ((__printf__, 4, 5)); > extern void __argp_failure (const struct argp_state *__restrict __state, > 7int __status, int __errnum, Especially there with this extra "7". -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
Re: [PATCH][BZ #16907] Sync argp.h __attribute__ with gnulib.
On Tue, Jun 03, 2014 at 01:28:00PM +0200, Ondřej Bílka wrote: > On Tue, Jun 03, 2014 at 12:59:11PM +0200, Aurelien Jarno wrote: > > On Fri, May 23, 2014 at 07:34:57PM +0200, Ondřej Bílka wrote: > > > On Fri, May 23, 2014 at 07:53:11AM -0700, Paul Eggert wrote: > > > > Ondřej Bílka wrote: > > > > >CCing gnulib. Could this be backported also there or do you have > > > > >different solution? > > > > > > > > We solved this problem in a different way years ago, using something > > > > like this: > > > > > > > > #if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR >= 7) > > > > # define _GL_ATTRIBUTE_FORMAT(s) __attribute__ ((__format__ s)) > > > > #else > > > > # define _GL_ATTRIBUTE_FORMAT(s) > > > > #endif > > > > > > > > and then using _GL_ATTRIBUTE_FORMAT ((__printf__, 2, 3)) later, > > > > instead of __attribute__ ((__format__ (__printf__, 2, 3))). > > > > Presumably glibc could do something similar. Glibc could use a > > > > different name; that's all right, gnulib will just switch to the > > > > name that glibc uses. > > > > > > Ok, here is patch that does that with unchanged name (Does somebody have > > > better one?). > > > > > > As I looked at header differences they are mostly minor, like using > > > `foo' instead "foo" in comments so it migth be worthwhile to sync them > > > up. > > > > > > > > > * argp/argp-fmtstream.h (_GL_ATTRIBUTE_FORMAT): Define. > > > (argp_error, argp_failure): Use _GL_ATTRIBUTE_FORMAT. > > > * argp/argp.h (__argp_fmtstream_printf): Likewise. > > > > Thanks for working on this issue. The patch looks fine to me, and I have > > just tested it, it fixes the original issue. Please also note that your > > patch doesn't apply cleanly, it seems there are some issue with the > > context. > > > Yes, I created that by taking a diff and removing extra parts so that > change man typo. A fixed version is here. Yes, I confirm it now applies cleanly. I think everything is ok now. Thanks, Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
Re: autoconf warning in gl_EARLY
Thanks, that worked. I put gl_EARLY at this location on the suggestion
of gnulib-tool.
2014-06-02 8:58 GMT-04:00 Eric Blake :
> On 06/01/2014 12:16 PM, Denis Laroche wrote:
>> I'm not sure if the post should be sent to the autoconf mailing list
>> instead. I see the following warnings from gl_EARLY when running
>> autoconf (version 2.69):
>
> The bug is on your end, not in gnulib.
>
>
>> The version of gnulib is 0.1.147-b1b4ba. Looking at the definition of
>> gl_EARLY, it's expanding macro gl_USE_SYSTEM_EXTENSIONS. The
>> documenation of this macro says that its purpose is to avoid those
>> warnings. The rest of the message shows the beginning of configure.ac.
>
> And for gl_EARLY to do its job, it MUST be called early.
>
>>
>> Thanks in advance!
>>
>> AC_PREREQ([2.69])
>>
>> AC_INIT([lvibs], [0.1.0], [lvibs...@gmail.ckom])
>> AC_CONFIG_AUX_DIR([build-aux])
>> AC_CONFIG_MACRO_DIR([m4])
>> AC_CONFIG_SRCDIR([src/lvibs.c])
>>
>> AM_INIT_AUTOMAKE([1.11 color-tests subdir-objects -Wall])
>> LT_PREREQ([2.2])
>
> call it here...
>
>> AM_PROG_AR
>> LT_INIT
>>
>> AM_MAINTAINER_MODE([enable])
>>
>> # Checks for programs.
>> AC_PROG_CC
>> AC_PROG_INSTALL
>> AM_PROG_CC_C_O
>>
>> # Checks for optional programs.
>> PROG_TRY_DOXYGEN
>>
>> AM_CONDITIONAL([HAVE_DOXYGEN], [test -n "$DOXYGEN"])
>> AS_IF([test -n "$DOXYGEN"], [AC_CONFIG_FILES([apidoc/doxyfile])])
>>
>> AC_LANG([C])
>>
>> AC_PROG_CC_C99
>> if test "$x{ac_cv_prog_cc_c99}" = xno
>> then
>> AC_MSG_FAILURE([
>>
>> A C99 compiler is required to build the
>> program.
>>
>> ])
>> fi
>>
>> gl_EARLY
>
> ...not here.
>
>> gl_INIT
>>
>>
>>
>
> --
> Eric Blake eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
[PATCH] mountlist: ensure hasmntopt's opt argument is passed with correct type
* Solaris defines hasmntop with a char * instead of const char * second argument. Passing the constant string "ignore" generates a compiler warning. Define MNT_IGNORE correctly in both cases to avoid the warning. Signed-off-by: Ben Walton --- lib/mountlist.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/mountlist.c b/lib/mountlist.c index 78af951..201d918 100644 --- a/lib/mountlist.c +++ b/lib/mountlist.c @@ -134,7 +134,13 @@ #undef MNT_IGNORE #ifdef MNTOPT_IGNORE -# define MNT_IGNORE(M) hasmntopt (M, MNTOPT_IGNORE) +# if (defined (__sun) && defined (__SVR4)) +/* Solaris defines hasmntopt(struct mnttab *, char *) + while it is otherwise hasmntopt(struct mnttab *, const char *) */ +# define MNT_IGNORE(M) hasmntopt (M, (char *) MNTOPT_IGNORE) +# else +# define MNT_IGNORE(M) hasmntopt (M, MNTOPT_IGNORE) +# endif #else # define MNT_IGNORE(M) 0 #endif -- 1.9.1
