Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |  

This web mail archive is frozen.

This page is part of a frozen web archive of this mailing list.

You can still navigate around this archive, but know that no new mails have been added to it since July of 2016.

Click here to be taken to the new web archives of this list; it includes all the mails that are in this frozen archive plus all new mails that have been sent to the list since it was migrated to the new archives.

Subject: Re: [OMPI devel] [OMPI svn-full] svn:open-mpi r24665
From: Larry Baker (baker_at_[hidden])
Date: 2011-05-02 14:17:00


Ralph,

What about creating a lookup table of static const values with
comments for readability, and use Tim's code, except for the last
line, which would lookup the value instead of computing it?

I don't know how often this code path is traversed. Seeing only this
snippet of code, I prefer Tim's code because it is clear what the
valid values are for the input argument (no need to scan all the
"case"s, find there is a "default", and deduce what the valid input
values are), it is more efficient in space and time, and, to my eyes,
more readable (I don't have to know what parse_dots() returns). I
suppose a case could also be made that Tim's code is more
maintainable, given the discovery already of a misplaced (though
benign) break and the possibility of a typo in all those calls to
parse_dots().

Just my $.02

Larry Baker
US Geological Survey
650-329-5608
baker_at_[hidden]

On May 1, 2011, at 7:44 AM, Ralph Castain wrote:

> Mostly because I thought it of some value to make the resulting mask
> readable and apparent to someone looking at the code.
>
>
> On Apr 30, 2011, at 8:14 PM, Tim Mattox wrote:
>
>> Why not do this instead of a big switch statement?
>>
>> pval = strtol(msk, NULL, 10);
>> if ((pval > 30) || (pval < 1)) {
>> opal_output(0, "opal_iftupletoaddr: unknown mask");
>> free(addr);
>> return OPAL_ERROR;
>> }
>> *mask = 0xFFFFFFFF << (32 - pval);
>>
>>
>> On Fri, Apr 29, 2011 at 1:56 PM, <rhc_at_[hidden]> wrote:
>>> Author: rhc
>>> Date: 2011-04-29 13:56:15 EDT (Fri, 29 Apr 2011)
>>> New Revision: 24665
>>> URL: https://svn.open-mpi.org/trac/ompi/changeset/24665
>>>
>>> Log:
>>> Cover all the netmask values
>>>
>>> Text files modified:
>>> trunk/opal/util/if.c | 103 ++++++++++++++++++++++++++++++++++++
>>> +--
>>> 1 files changed, 96 insertions(+), 7 deletions(-)
>>>
>>> Modified: trunk/opal/util/if.c
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> ====================================================================
>>> --- trunk/opal/util/if.c (original)
>>> +++ trunk/opal/util/if.c 2011-04-29 13:56:15 EDT (Fri, 29
>>> Apr 2011)
>>> @@ -534,13 +534,102 @@
>>> * much of the addr to use: e.g., /16
>>> */
>>> pval = strtol(msk, NULL, 10);
>>> - if (24 == pval) {
>>> - *mask = 0xFFFFFF00;
>>> - } else if (16 == pval) {
>>> - *mask = 0xFFFF0000;
>>> - } else if (8 == pval) {
>>> - *mask = 0xFF000000;
>>> - } else {
>>> + switch(pval) {
>>> + case 30:
>>> + *mask = parse_dots("255.255.255.252");
>>> + break;
>>> + case 29:
>>> + *mask = parse_dots("255.255.255.248");
>>> + break;
>>> + case 28:
>>> + *mask = parse_dots("255.255.255.240");
>>> + break;
>>> + case 27:
>>> + *mask = parse_dots("255.255.255.224");
>>> + break;
>>> + case 26:
>>> + *mask = parse_dots("255.255.255.192");
>>> + break;
>>> + case 25:
>>> + *mask = parse_dots("255.255.255.128");
>>> + break;
>>> + case 24:
>>> + break;
>>> + *mask = parse_dots("255.255.255.0");
>>> + break;
>>> + case 23:
>>> + *mask = parse_dots("255.255.254.0");
>>> + break;
>>> + case 22:
>>> + *mask = parse_dots("255.255.252.0");
>>> + break;
>>> + case 21:
>>> + *mask = parse_dots("255.255.248.0");
>>> + break;
>>> + case 20:
>>> + *mask = parse_dots("255.255.240.0");
>>> + break;
>>> + case 19:
>>> + *mask = parse_dots("255.255.224.0");
>>> + break;
>>> + case 18:
>>> + *mask = parse_dots("255.255.192.0");
>>> + break;
>>> + case 17:
>>> + *mask = parse_dots("255.255.128.0");
>>> + break;
>>> + case 16:
>>> + *mask = parse_dots("255.255.0.0");
>>> + break;
>>> + case 15:
>>> + *mask = parse_dots("255.254.0.0");
>>> + break;
>>> + case 14:
>>> + *mask = parse_dots("255.252.0.0");
>>> + break;
>>> + case 13:
>>> + *mask = parse_dots("255.248.0.0");
>>> + break;
>>> + case 12:
>>> + *mask = parse_dots("255.240.0.0");
>>> + break;
>>> + case 11:
>>> + *mask = parse_dots("255.224.0.0");
>>> + break;
>>> + case 10:
>>> + *mask = parse_dots("255.192.0.0");
>>> + break;
>>> + case 9:
>>> + *mask = parse_dots("255.128.0.0");
>>> + break;
>>> + case 8:
>>> + *mask = parse_dots("255.0.0.0");
>>> + break;
>>> + case 7:
>>> + *mask = parse_dots("254.0.0.0");
>>> + break;
>>> + case 6:
>>> + *mask = parse_dots("252.0.0.0");
>>> + break;
>>> + case 5:
>>> + *mask = parse_dots("248.0.0.0");
>>> + break;
>>> + case 4:
>>> + *mask = parse_dots("240.0.0.0");
>>> + break;
>>> + case 3:
>>> + *mask = parse_dots("224.0.0.0");
>>> + break;
>>> + case 2:
>>> + *mask = parse_dots("192.0.0.0");
>>> + break;
>>> + case 1:
>>> + *mask = parse_dots("128.0.0.0");
>>> + break;
>>> + case 0:
>>> + *mask = parse_dots("0.0.0.0");
>>> + break;
>>> + default:
>>> opal_output(0, "opal_iftupletoaddr: unknown
>>> mask");
>>> free(addr);
>>> return OPAL_ERROR;
>>> _______________________________________________
>>> svn-full mailing list
>>> svn-full_at_[hidden]
>>> http://www.open-mpi.org/mailman/listinfo.cgi/svn-full
>>>
>>
>>
>>
>> --
>> Tim Mattox, Ph.D. - http://homepage.mac.com/tmattox/
>> timattox_at_[hidden] || tmattox_at_[hidden]
>> I'm a bright... http://www.the-brights.net/
>>
>> _______________________________________________
>> devel mailing list
>> devel_at_[hidden]
>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>
>
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel