Login | Register
My pages Projects Community openCollabNet

Discussions > dev [DISABLED] > Re: [PATCH] Remove warning about code not beeing executed

subversion
Discussion topic

Back to topic list

Re: [PATCH] Remove warning about code not beeing executed

Author julianfoad
Full name Julian Foad
Date 2009-11-05 10:50:26 PST
Message Stefan Sperling wrote:
> On Thu, Nov 05, 2009 at 07:28:18PM +0100, Daniel Näslund wrote:
> > static void
> > wrap_pre_blame3_rece​iver(svn_client_blam​e_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" */
[...]

« Previous message in topic | 3 of 7 | Next message in topic »

Messages

Show all messages in topic

[PATCH] Remove warning about code not beeing executed dannas Daniel Näslund 2009-11-05 10:28:26 PST
     Re: [PATCH] Remove warning about code not beeing executed stsp Stefan Sperling 2009-11-05 10:35:04 PST
         Re: [PATCH] Remove warning about code not beeing executed julianfoad Julian Foad 2009-11-05 10:50:26 PST
             Re: [PATCH] Remove warning about code not beeing executed Daniel Shahaf <d dot s at daniel dot shahaf dot name> Daniel Shahaf <d dot s at daniel dot shahaf dot name> 2009-11-05 10:58:23 PST
                 Re: [PATCH] Remove warning about code not beeing executed julianfoad Julian Foad 2009-11-05 11:23:59 PST
         Re: [PATCH] Remove warning about code not beeing executed brane Branko Cibej 2009-11-05 13:25:23 PST
             Re: [PATCH] Remove warning about code not beeing executed dlr Daniel Rall 2009-11-05 16:33:27 PST
Messages per page: