httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: Coding style
Date Mon, 02 Oct 2006 20:39:32 GMT


On 10/02/2006 08:59 PM, Garrett Rooney wrote:
> On 10/2/06, Nick Kew <nick@webthing.com> wrote:
> 
>> We have a bunch of new bug reports[1], detailing bugs of the form
>>
>>    if ((rv = do_something(args) == APR_SUCCESS))
>>
>> for
>>
>>     if ((rv = do_something(args)) == APR_SUCCESS)
>>
>> Of course, that's a C classic, and can be a *** to spot.
>>
>> We can avoid this by adopting an alternative coding style
>> that doesn't rely on confusing parentheses:
>>
>>   if (rv = do_something(args), rv == APR_SUCCESS)
>>
>> Can I suggest adopting this as a guideline for new code,
>> to avoid this kind of bug?
> 
> 
> Or the even more readable:
> 
> rv = do_something(args);
> if (rv == APR_SUCCESS) {
> 
> }

In general I agree with the above, but there are situations were the old
style really makes sense, e.g (from log_child in server/log.c):

if (((rc = apr_procattr_create(&procattr, p)) == APR_SUCCESS)
        && ((rc = apr_procattr_cmdtype_set(procattr,
                                           APR_SHELLCMD_ENV)) == APR_SUCCESS)
        && ((rc = apr_procattr_io_set(procattr,
                                      APR_FULL_BLOCK,
                                      APR_NO_PIPE,
                                      APR_NO_PIPE)) == APR_SUCCESS)
        && ((rc = apr_procattr_error_check_set(procattr, 1)) == APR_SUCCESS)
        && ((rc = apr_procattr_child_errfn_set(procattr, log_child_errfn)) == APR_SUCCESS))
{

To get the same thing with above style you would create a lot
of nested if statements or you would have to use a lot of if statements and goto.

Regards

RĂ¼diger

Mime
View raw message