[openib-general] [PATCH 3/4] New routing module which loads LFT tables from dump file.

Hal Rosenstock halr at voltaire.com
Wed Jun 14 10:20:53 PDT 2006


Hi Eitan,

On Wed, 2006-06-14 at 02:48, Eitan Zahavi wrote:
> Hi Hal, Sasha,
> 
> Regarding OpenSM coding style:
> 
> Sasha wrote:
> > 
> > Really? Don't want to bother with examples, but I may see almost any
> > "combination" in OpenSM and it is not clear for me which one is common
> > (the coding style and identation are different even from file to
> file).
> [EZ] This bothers me as I think we should use a consistent coding style.
> You might also remember we had put in place a both a script to do
> automatic indentation and coding style rule fixes (osm_indent and
> osm_check_n_fix)
> 
> I did check for all "else" statements:
> osm/opensm>grep else *.c | wc -l
> 397
> osm/opensm>grep else *.c | grep -v "{" | grep -v "}" | wc -l
> 361
> 
> So you can see only <10%  (36 out of 397) "else" statement are not
> coding style consistent. 
> Checking what is the code that is "non standard":
> osm/opensm>grep else *.c | grep "{" | awk '{print $1}' | sort | uniq -c
> | sort -rn
>       7 osm_console.c:
>       6 osm_prtn_config.c:
>       3 st.c:
>       3 osm_sa_multipath_record.c:
>       2 osm_ucast_mgr.c:
>       2 osm_sa_path_record.c:
>       1 osm_sa_mcmember_record.c:
>       1 osm_sa_informinfo.c:
>       1 osm_sa_class_port_info.c:
>       1 osm_multicast.c:
> 
> You can see the majority of these mismatches are in code introduced by
> Hal and yourself.

While some of those are ours (clearly osm_console.c, osm_prtn_config.c,
and osm_sa_multipath_record.c), not all of them are. I'm sure some came
from you, Yael, and Ofer so let's not be pointing fingers. I don't
bother to kick back each patch on these details. If I did, we would get
no where. I fixed a number of the ones you pointed to above just now.

But let's back up a bit...

> I think OpenSM should sue a single code style. 

This is the key but now is (still) not the time. How about we take this
up in about a month maybe sooner if things settle down a little quicker
? I'll bring this up on the list when I think the time is right. I do
think it will take time to agree on this and a lot of the rules will be
arbitrary.

> My proposal is that we
> update our osm_indent script with a set of rules we agree on and apply
> to the entire tree.

I'm unconvinced that osm_indent is sufficient. I think a lot of human
attention is needed afterwards. I've seen that happen before. How much
time do you have to invest in doing this ?

-- Hal






More information about the general mailing list