[PATCH 3/16] add base support for Celleb platform

Ishizaki Kou kou.ishizaki at toshiba.co.jp
Fri Nov 17 21:27:54 EST 2006


Hello Christoph-san,

> > +         if(size-- <= 0) {
> > +             return NULL;
> > +         }

> No need for the {} braces for single line conditionals.  also
> please put a space in front of opening braces.  If you're unsure
> please take a look at Documentation/CodingStyle in the kernel tree.

> > + if((str = re_det1(str, &buf[0].cbuf[4], 4)) == NULL) {
> > +     return -1;
> > + }

> Similarly this should be written as:

>     str = re_det1(str, &buf[0].cbuf[4], 4);
>     if (!str)
>         return -1;

Thank you for your comment; these will be fixed at next time.

> And btw, all these function have very non-descriptive names
> and no comments.  That could use some work.  Also I'm not
> sure -N returns are very useful - you should at least have
> descriptive enums for the error cases.

We don't think these functions are useful for all kernel users,
since they work only on Beat. And, for debugging purpose, return values
indicate the locations of errors in above case, so we don't have enums
for these error cases. But it isn't easy to understand them without
comments as you mentioned, so we will add adequate comments at next time.

Thank you,
Kou Ishizaki
Toshiba



More information about the Linuxppc-dev mailing list