Stefan Sperling wrote:
> On Thu, Nov 05, 2009 at 07:28:18PM +0100, Daniel Näslund wrote:
> > static void
> > wrap_pre_blame3_receiver(svn_client_blame_receiver_t *receiver,
> > - void **receiver_baton,
> > + void **receiver_baton, const char *eol,
> > apr_pool_t *pool)
> > {
> > - if (strlen(APR_EOL_STR) > 1)
> > + if (strlen(eol) > 1)
>
> No idea if we really need to fix this.
The guiding principle is: never make code more complex without a VERY
good reason. It's difficult enough as it is.
Avoiding that warning is not, I think, a good enough reason to make that
change.
I think the ultimate analysis here is that the compiler doesn't know
whether APR_EOL_STR ever has a different value. It warns because, in the
current translation unit, that code will never be executed; but it
doesn't know that sometimes we might ask it to compile with a different
APR_EOL_STR. So, the problem here is that the warning is based on
insufficient information. Maybe the next version of GCC will have a way
to indicate that, or maybe it will stop issuing that warning in cases
like this, or ... Anyway, if the warning is not very good, not very
helpful, then we should disable it rather than twisting the code to
avoid triggering it. After all, with your patch to pass the string as a
parameter, a whole-file-optimiser could optimise out your transformation
and still generate the same warning.
- Julian
> There's a nice snippet in libsvn_subr/prompt.c which generates a similar
> warning but avoids strlen():
>
> /* GCC might complain here: "warning: will never be executed"
> * That's fine. This is a compile-time check for "\r\n\0" */
[...]
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414874
Received on 2009-11-05 19:50:38 CET