Actually, that is not entirely accurate. There is the possibility for
compiler optimization based on the ternary operator.
Brian
Jeff Squyres wrote:
> I've always been a verbose guy :-), but I don't really care too much
> in this situation (the patch is style, not function). I'll apply
> Paul's suggestion.
>
> Thanks guys.
>
>
> On Mar 12, 2008, at 10:42 PM, Paul H. Hargrove wrote:
>
>
>> I find the resulting line too be hard to read, and prone to misparsing
>> by a human. Mixing assignment and conditionals in the same statement
>> has always been something I've tried to avoid due to the possibility
>> of
>> confusion.
>> I suggest keeping the "ret = syscall(...)" lines unchanged, and only
>> replacing the if/else constructs with
>> return (ret >= 0) ? 0 : ret;
>> or
>> return (ret < 0) ? ret : 0;
>> Where, to me, the second option stresses that negative "ret" is
>> preserved, while the first stresses that positive "ret" are "squashed"
>> to zero. Not sure which one wants to emphasize (or even that anyone
>> else reads these as I do.).
>> Other than that, I agree w/ Brian that the current if/else is too
>> much.
>>
>> -Paul
>>
>> Brian Curtis wrote:
>>
>>> Below is a small patch to plpa_dispatch.c to avoid verbose if-else
>>> statements.
>>>
>>>
>>> Brian
>>>
>>>
>>> --- plpa_dispatch.c 2008-03-11 11:59:23.000000000 -0400
>>> +++ plpa_dispatch.c 2008-03-12 23:05:56.000000000 -0400
>>> @@ -110,17 +110,11 @@ int PLPA_NAME(sched_setaffinity)(pid_t p
>>> memcpy(&tmp, cpuset, cpusetsize);
>>> }
>>>
>>> - /* Now do the syscall */
>>> - ret = syscall(__NR_sched_setaffinity, pid, PLPA_NAME(len),
>>> &tmp);
>>> -
>>> - /* Return 0 upon success. According to
>>> + /* Now do the syscall:
>>> + Return 0 upon success. According to
>>> http://www.open-mpi.org/community/lists/plpa-users/
>>> 2006/02/0016.php,
>>> all the kernel implementations return >= 0 upon
>>> success. */
>>> - if (ret >= 0) {
>>> - return 0;
>>> - } else {
>>> - return ret;
>>> - }
>>> + return (ret = syscall(__NR_sched_setaffinity, pid, PLPA_NAME
>>> (len), &tmp)) >= 0 ? 0 : ret;
>>> break;
>>>
>>> case PLPA_NAME_CAPS(PROBE_NOT_SUPPORTED):
>>> @@ -184,17 +178,11 @@ int PLPA_NAME(sched_getaffinity)(pid_t p
>>> memset(cpuset, 0, cpusetsize);
>>> }
>>>
>>> - /* Now do the syscall */
>>> - ret = syscall(__NR_sched_getaffinity, pid, PLPA_NAME(len),
>>> cpuset);
>>> -
>>> - /* Return 0 upon success. According to
>>> + /* Now do the syscall:
>>> + Return 0 upon success. According to
>>> http://www.open-mpi.org/community/lists/plpa-users/
>>> 2006/02/0016.php,
>>> all the kernel implementations return >= 0 upon
>>> success. */
>>> - if (ret >= 0) {
>>> - return 0;
>>> - } else {
>>> - return ret;
>>> - }
>>> + return (ret = syscall(__NR_sched_getaffinity, pid, PLPA_NAME
>>> (len), cpuset)) >= 0 ? 0 : ret;
>>> break;
>>>
>>> case PLPA_NAME_CAPS(PROBE_NOT_SUPPORTED):
>>>
>>> _______________________________________________
>>> plpa-users mailing list
>>> plpa-users_at_[hidden]
>>> http://www.open-mpi.org/mailman/listinfo.cgi/plpa-users
>>>
>>>
>> --
>> Paul H. Hargrove PHHargrove_at_[hidden]
>> Future Technologies Group
>> HPC Research Department Tel: +1-510-495-2352
>> Lawrence Berkeley National Laboratory Fax: +1-510-486-6900
>>
>>
>> _______________________________________________
>> plpa-users mailing list
>> plpa-users_at_[hidden]
>> http://www.open-mpi.org/mailman/listinfo.cgi/plpa-users
>>
>
>
>
|