Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-28 Thread Bart Van Assche
On Wed, 2018-11-28 at 17:44 +0100, David Disseldorp wrote: > Hi Bart, > > On Wed, 28 Nov 2018 08:36:19 -0800, Bart Van Assche wrote: > > > Maybe I'm missing something, but why is zeroing of unused bytes in these > > functions > > necessary? Would the following be correct if all strings in struct

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-28 Thread David Disseldorp
Hi Bart, On Wed, 28 Nov 2018 08:36:19 -0800, Bart Van Assche wrote: > Maybe I'm missing something, but why is zeroing of unused bytes in these > functions > necessary? Would the following be correct if all strings in struct t10_wwn > would be > '\0'-terminated? Your patch looks good to me. Min

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-28 Thread Bart Van Assche
On Wed, 2018-11-28 at 17:28 +0100, David Disseldorp wrote: > On Wed, 28 Nov 2018 08:08:30 -0800, Bart Van Assche wrote: > > > > Just a follow up here. I think it's safer to retain strncpy() in this > > > function for the purpose of zero filling. scsi_dump_inquiry() and > > > target_stat_lu_vend_sh

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-28 Thread David Disseldorp
On Wed, 28 Nov 2018 08:08:30 -0800, Bart Van Assche wrote: > > Just a follow up here. I think it's safer to retain strncpy() in this > > function for the purpose of zero filling. scsi_dump_inquiry() and > > target_stat_lu_vend_show() walk the entire length of the t10_wwn.vendor > > buffer. > >

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-28 Thread Bart Van Assche
On Wed, 2018-11-28 at 16:37 +0100, David Disseldorp wrote: > On Tue, 20 Nov 2018 19:00:57 +0100, David Disseldorp wrote: > > > > > + strncpy(buf, page, sizeof(buf)); > > > > > > Isn't strncpy() deprecated? How about using strlcpy() instead? > > > > Will change to use strlcpy in the n

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-28 Thread David Disseldorp
On Tue, 20 Nov 2018 19:00:57 +0100, David Disseldorp wrote: > > > + strncpy(buf, page, sizeof(buf)); > > > > Isn't strncpy() deprecated? How about using strlcpy() instead? > > Will change to use strlcpy in the next round. Just a follow up here. I think it's safer to retain strncpy() in th

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-20 Thread Bart Van Assche
On Tue, 2018-11-20 at 19:00 +0100, David Disseldorp wrote: > I tend to agree that it's dangerous, but chose to stay somewhat > consistent with the other t10_wwn strings that are treated as though > they may not be NULL terminated. > > If you're in favour adding an extra terminator byte here, then

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-20 Thread David Disseldorp
On Tue, 20 Nov 2018 09:24:39 -0800, Bart Van Assche wrote: > On Mon, 2018-11-19 at 22:06 +0100, David Disseldorp wrote: > > /* > > + * STANDARD and VPD page 0x80 T10 Vendor Identification > > + */ > > +static ssize_t target_wwn_vendor_id_show(struct config_item *item, > > + char *page)

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-20 Thread Bart Van Assche
On Mon, 2018-11-19 at 22:06 +0100, David Disseldorp wrote: > /* > + * STANDARD and VPD page 0x80 T10 Vendor Identification > + */ > +static ssize_t target_wwn_vendor_id_show(struct config_item *item, > + char *page) > +{ > + return sprintf(page, "T10 Vendor Identification: %." > +

[PATCH v3 3/4] target: add device vendor_id configfs attribute

2018-11-19 Thread David Disseldorp
The vendor_id attribute will allow for the modification of the T10 Vendor Identification string returned in inquiry responses. Its value can be viewed and modified via the ConfigFS path at: target/core/$backstore/$name/wwn/vendor_id "LIO-ORG" remains the default value, which is set when the backst