Re: [PATCH 01/13] IB/core: Add get FW version string to the core

["ira.weiny" <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

On Thu, Jun 09, 2016 at 04:29:19PM -0600, Jason Gunthorpe wrote:
> On Thu, Jun 09, 2016 at 05:53:56PM -0400, ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> > +void ib_get_device_fw_str(struct ib_device *dev, char *str, size_t str_len)
> > +{
> > +	if (dev->get_dev_fw_str)
> > +		dev->get_dev_fw_str(dev, str, str_len);
> > +	else
> > +		snprintf(str, str_len, "%s", "");
> 
> That is the strangest way to write str[0] = 0;

I find it to be very explicit without any performance requirements.

How about?

str[0] = '\0';

> 
> >  void ib_dealloc_device(struct ib_device *device);
> >  
> > +void ib_get_device_fw_str(struct ib_device *device, char *, size_t);
> 
> It is not common in the kernel to drop the parameter names from the
> prototype, please don't add more.

Good catch sorry about that.

Fixed.

Ira

> 
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html