[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [patch] fix nits in recent cwrapper patch
From: |
Eric Blake |
Subject: |
Re: [patch] fix nits in recent cwrapper patch |
Date: |
Fri, 8 Jun 2007 18:36:07 +0000 (UTC) |
User-agent: |
Loom/3.14 (http://gmane.org/) |
Charles Wilson <libtool <at> cwilson.fastmail.fm> writes:
>
> Hopefully the attached addresses all of the concerns mentions by Noah,
> Erick, and Peter with regards to my recent cwrapper-emits-shwrapper patch.
s/Erick/Eric/, but that's okay (I've seen worse butchering of my name, as short
as it is, and I'm sure I've done worse to those with longer names :)
> 2007-06-08 Charles Wilson <...>
>
> (func_emit_libtool_cwrapperexe_source) [file scope]:
> define permission flags S_IXGRP and S_IXOTH if not
> already defined.
> (func_emit_libtool_cwrapperexe_source) [LTWRAPPER_DEBUGPRINTF]:
> declare as a function, not a macro, as variadic macros
> are not supported by C89.
This means that all arguments to LTWRAPPER_DEBUGPRINTF are now evaluated, even
when before in a non-debug build they were not. But I don't think that's a
show-stopper, though, as long as we aren't doing something stupid in
calculating those arguments. And a quick glance didn't turn anything up.
> (func_emit_libtool_cwrapperexe_source) [check_executable]:
> avoid embedded #ifdefs; use S_IXGRP and S_IXOTH
> unconditionally.
> (func_emit_libtool_cwrapperexe_source) [make_executable]:
> ditto.
Looked right to me.
>
> -#undef LTWRAPPER_DEBUGPRINTF
> +void LTWRAPPER_DEBUGPRINTF(const char* fmt, ...)
One last nit - why are we exporting this? Make it look like this:
static void
LTWRAPPER_DEBUGPRINTF(const char* fmt, ...)
and then I'm okay with your resolution to the nits that I brought up.
[As far as I know, the only way in C89 to get variadic macro behavior that can
be entirely elided is with stuff like
#if DEBUG
# define WRAPPER(args) mywrapper args
int mywrapper(char*format, ...);
#else
# define WRAPPER(args)
#endif
WRAPPER((arg1, arg2))
but that means changing all callers, whereas simply trading C99 variadic macro
for a C89 variadic function only changes the definition, at the expense of the
semantic change of always evaluating the arguments.]
--
Eric Blake