Artem Kachitchkine wrote:
>
>> http://cr.opensolaris.org/~gdamore/blk2scsa/
>
> I won't have time to dig deep and verify logic, here's something that
> caught the eye during a quick browse:
>
> blk2scsa.h:
>
> Would be nice to use enum for constants instead of #define: that way,
> they would be included in CTF and make debugging (even) more pleasant.
Hmm... that's not a bad idea. I didn't know that CTF debugging worked
better for enums. I guess the errno and commands could be done that
way. (I am always a bit uncomfortable using enums where I care about
the value. Here I don't care about the value, other than the value
cannot change... i.e. deleting a value could cause breakage in the API.)
>
> blk2scsa.c:
>
> 142 static struct modlinkage modlinkage = {
> 143 MODREV_1, { &modlmisc, NULL }
>
> This gotta be the only ON driver that uses {} syntax for linkage array :)
Heh. I think afe.c also uses it. But then again, I wrote that, too.
:-) The reason I use that syntax is that some other validation tool I
used at one point complained about the missing braces. It might have
been an older version of gcc with -Wall -Wpedantic or somesuch. The
braces really *do* belong there. ;-)
>
> Have you considered adding warlock instrumentation? Most our SCSA
> modules are warlockable individually and together
> (framework+adapter+target).
I thought about it. Part of the reason I didn't was because the warlock
rules looked a bit hackish in the Makefiles. It seems to me that the
way they were added was not terribly well thought out.
Anyway, if there is pressure to do that, I could do so, but I don't want
to stall this project on it at this time. Warlock isn't a tool I'm
familiar with.
-- Garrett
>
> -Artem
>
>
