On Thu, Apr 26, 2007 at 04:55:14PM +1000, Dave Airlie wrote:
A few easy and simple comments based on looking at this for 5 minutes:
- drop the typedefs. Yeah, they might be a drm thing, but we don't
need them here.
- the comment style for the functions is "odd" and not in kerneldoc
form, but something else. Either use kerneldoc or nothing, don't
invent something new please.
- what's with the /proc interface? Don't add new proc code for
non-process related things. This should all go into sysfs
somewhere. And yes, I know /proc/dri/ is there today, but don't add
new stuff please.
- struct drm_bo_arg can't use an int or unsigned, as it crosses the
userspace/kernelspace boundry, use the proper types for all values
in those types of structures (__u32, etc.)
- there doesn't seem to be any validity checking for the arguments
passed into this new ioctl. Possibly that's just the way the rest
of the dri interface is, which is scary, but with the memory stuff,
you really should check things properly...
that's enough to start with :)
thanks,
greg k-h
-
