On Sun, Nov 2, 2008 at 8:28 PM, Peter Buckingham
<[EMAIL PROTECTED]> wrote:
> Hi Cyril,
>
> Cyril Plisko wrote:
>>
>> I am recently spending quite some time getting acquainted with COMSTAR
>> source code and it seems that the code could use some cleanups. Both
>> cstyle and various small non-cstyle.
>> Is there any interest in such cleanups ? If so I am willing to
>> contribute it - this work has a nice side effect of helping me
>> understand code better :)
>
> I'd suggest sending the diffs along and we can work through them.
>

Peter,

ok, here we go:

the web rev is http://cr.opensolaris.org/~imp/comstar
There are five webrevs there, each successive one is an incremental
against the previous - I have these as different patches in my MQ. Why
is it broken up into 5 patches ? I wanted it to be grouped by the type
of the problem/thing it fixes. I think it will make the review easier.

So, what do we have there ?

o 01-cstyle-indentation-fixes.patch

This patch fixes almost all the cstyle indentation complains. This
patch doesn't change the code except reformatting it.
There are quite a bunch of those.

o 02-cstyle-destructive-indentation-fixes.patch

This patch complements the work of the previous one on the cstyle
cleanness. However, in order to fix these I had to change the code a
bit. Fortunately, there is only one place like that (discovery.c). I
used that opportunity to reduce the number of dereferences used in the
code and it had the nice side effect of compacting the code and making
the cstyle happy.

o 03-kmem_alloc.patch

This patch changes calls to kmem_alloc() to kmem_zalloc(). I believe
it is cleaner (the buffers coming from kmem_alloc() are deterministic)
that way and in one place it helps simplifying the code somewhat (see
stmf_sbd.c:867)

o 04-snprintf-buffer-fixes.patch

This patch eliminates unneeded manual NULL-termination of the string
produced by snprintf() - they are already NULL-termiated. It also
changes the buffer size numbers used throughout the code into
#define'd constants.

o 05-stmf-include.patch

The last one is taking advantage of the fact that the code lives in ON
repo now and doesn't require special -I directives. All the headers
are in standard place and should be readily accessible. Since these
headers are delivered with SXCE build it also helps 3rd party drivers
to build without adding -I/usr/include/sys to their CFLAGS.

Feedback on these patches is greatly appreciated.

-- 
Regards,
        Cyril
_______________________________________________
storage-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/storage-discuss

Reply via email to