On Mon, Sep 15, 2014 at 12:14:49PM -0500, Romer, Benjamin M wrote:
> > Have you actually ran the checkpatch.pl tool on this code? You still
> > have a lot of cleanup to do (hint, typedefs for drivers are not
> > allowed...)
>
> Yes, I've been using checkpatch.pl a lot, though admittedly I did not
> know about --strict. I'll start addressing the check issues as well as
> the warnings and errors.
>
> About the "WARNING: do not add new typedefs" messages that are
> generated, I have a question about what typedefs are permitted. It's
> easy enough to replace "typedef enum {...} x;" with "enum x {...}", but
> there are a lot of typedef struct declarations throughout the s-Par
> driver code used to give clearer names to internally-used structures. I
> don't know of any kernel-defined types being renamed anymore, and when I
> did a grep of the driver tree to see what other drivers were doing in
> this area, I found lots of drivers with "typedef struct" in them.
>
> Are we restricted from doing *any* typedefs at all?
Pretty much.
> If not, could you give me a good guideline to follow?
http://yarchive.net/comp/linux/typedefs.html
This one is probably ok, except the name is crap:
typedef u64 GUEST_PHYSICAL_ADDRESS;
I hate that the names are all caps.
Also We might have a standard type for this? I forget.
For structs, just use struct name throughout. I do understand that you
want many of them to be opaque, but really it's a struct. Just do a
forward declaration in the .h file and leave the implementation in the
.c file as you have it.
This file is really gross.
drivers/staging/unisys/common-spar/include/vmcallinterface.h
The comments and the pragmas.
This one is misleading. Normally in the kernel bool is a C _Bool type.
#define BOOL int
I don't think we actually use the SPARSTOP_COMPLETE_FUNC typedef. I
think function pointer typedefs are worthwhile because they are less
typing but this one can just be deleted obviously.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
