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

Reply via email to