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
--
Jeff Squyres
Cisco Systems
|
![[Flag]](../../../../../community/mirrors/flag-us.gif){kind=small}