[openib-general] [PATCH 2 of 3] mad: large RMPP support

[Roland Dreier]

 > +		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.