PLPA Users' Mailing List Archives

Subject: Re: [PLPA users] Patch to remove verbose if-else
From: Paul H. Hargrove (PHHargrove_at_[hidden])
Date: 2008-03-12 23:42:44

• Next message: Jeff Squyres: "Re: [PLPA users] Patch to remove verbose if-else"
• Previous message: Brian Curtis: "[PLPA users] Patch to remove verbose if-else"
• In reply to: Brian Curtis: "[PLPA users] Patch to remove verbose if-else"
• Next in thread: Jeff Squyres: "Re: [PLPA users] Patch to remove verbose if-else"
• Reply: Jeff Squyres: "Re: [PLPA users] Patch to remove verbose if-else"

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

• Next message: Jeff Squyres: "Re: [PLPA users] Patch to remove verbose if-else"
• Previous message: Brian Curtis: "[PLPA users] Patch to remove verbose if-else"
• In reply to: Brian Curtis: "[PLPA users] Patch to remove verbose if-else"
• Next in thread: Jeff Squyres: "Re: [PLPA users] Patch to remove verbose if-else"
• Reply: Jeff Squyres: "Re: [PLPA users] Patch to remove verbose if-else"