> + rmpp_mad = (struct ib_rmpp_mad *)seg_buf->mad; Trivial, but I prefer a space after cast operators.
> +static struct ib_umad_packet *alloc_packet(void) > +{ > + struct ib_umad_packet *packet; > + int length = sizeof *packet + sizeof(struct ib_mad); > + > + packet = kzalloc(length, GFP_KERNEL); > + if (!packet) { > + printk(KERN_ERR "alloc_packet: mem alloc failed for length > %d\n", > + length); > + return NULL; > + } > + INIT_LIST_HEAD(&packet->seg_list); > + return packet; > +} This seems a little too big for what it's actually doing. Also, do we really need to print a kernel error when the system is out of memory? I would just write this as: static struct ib_umad_packet *alloc_packet(void) { struct ib_umad_packet *packet; packet = kzalloc(length, GFP_KERNEL); if (!packet) return NULL; INIT_LIST_HEAD(&packet->seg_list); return packet; } Also the chunk that deletes the old definition of alloc_packet() seems to be in the next patch, so the tree won't compile without both patches (which will be a pain from someone doing git bisect). > + /* User buffer too small. Return first RMPP segment (which > + * includes RMPP message length). > + */ Trivial again but please use the comment format /* * foo */ in other words put the opening "/*" on a line by itself. > + /* multipacket RMPP MAD message. Copy remainder of message. > + * Note that last segment may have a shorter payload. > + */ same here - R. _______________________________________________ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general