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

Eitan Zahavi eitan at mellanox.co.il
Wed Jun 14 12:17:06 PDT 2006


Hi Hal,

My point is clear: the fact there are some files that are inconsistent
should not open the door for a total mess. I think the simple statistics
do answer the question raised:
" ... and it is not clear for me which one is common".

If we do delay the major cleanup we should at least try to stick to the
existing standard.
>From the mail thread it seems the intention is different and this makes
me really uncomfortable.

Eitan

> 
> 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