- if (WARN_ON(vmf->pgoff >= ubuf->pagecount))
return VM_FAULT_SIGBUS;Just curious, when do you expect this to happen ?
It should not. If it actually happens it would be a bug somewhere, thats why the WARN_ON.
But you seem to consider that this condition that should never happen still has a high enough chance of happening that it's worth a WARN_ON(). I was wondering why this one in particular, and not other conditions that also can't happen and are not checked through the code.
Added it while writing the code, to get any coding mistake I make flagged right away instead of things exploding later on.
I can drop it.
- ubuf = kzalloc(sizeof(struct udmabuf), GFP_KERNEL);
sizeof(*ubuf)
Why? Should not make a difference ...
Because the day we replace
struct udmabuf *ubuf;
with
struct udmabuf_ext *ubuf;
and forget to change the next line, we'll introduce a bug. That's why sizeof(variable) is preferred over sizeof(type). Another reason is that I can easily see that
ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL);
is correct, while using sizeof(type) requires me to go and look up the declaration of the variable.
So it simplifies review, ok, will change it.
BTW: Maybe the kernel should pick up a neat trick from glib:
g_new0() is a macro which takes the type instead of the size as first argument, and it casts the return value to that type. So the compiler will throw warnings in case of a mismatch. That'll work better than depending purely on the coder being careful and review catching the remaining issues.
cheers, Gerd