Just looking over this patch, it would seem a few things are off a bit:
-= 1 =-
- if ((cgc.buffer = (char *)kmalloc(CD_FRAMESIZE_RAW*ra.nframes,
+ if ((cgc.buffer = (char *)kmalloc(CD_FRAMESIZE_RAW*8,
doesn't this change it from kmalloc'ing a ra.nframes frame block to an 8
frame block?
-= 2 =-
+ int nr = ra.nframes > 8 ? 8 : ra.nframes;
why are you adding another variable? If this is neccessary (glancing at
the code, it doesn't seem to be, just needs to be refined a bit),
shouldn't it be a static int? i'm guessing at least that gcc isn't going
to like you declaring a new int in the middle of things...
-= 3 =-
Yo forgot to modify the comment to "do between 1 and 300 frames at a
time".
-=-=-
<sarcasm>maybe we should make this a config option!</sarcasm>
though i must ask, why don't we pre-allocate cgc.buffer? i understand that
there's the memory loss, but i wouldn't mind it much myself. i'm not sure
how expensive kmalloc and kfree are, which is kind of important in
deciding this.
anyway, i could be totally unfounded on the first two, as i'm still
pretty new to kernel development, so feel free to tell me what manuals to
go read that show why these are silly points, etc =)
-- /jbm, but you can call me Josh. Really, you can. "If it looks good, you'll see it. If it sounds good, you'll hear it. If it's marketed right, you'll buy it. But... if it's real, You'll Feel It." -- Kid Rock
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/