| Message ID | 1509703370-20379-4-git-send-email-javier@cnexlabs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 03, 2017 at 11:02:50AM +0100, Javier González wrote: > Signed-off-by: Javier González <javier@cnexlabs.com> > --- > drivers/nvme/host/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index ae8ab0a1ef0d..f05c81774abf 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -2108,7 +2108,7 @@ static ssize_t eui_show(struct device *dev, struct device_attribute *attr, > char *buf) > { > struct nvme_ns *ns = nvme_get_ns_from_dev(dev); > - return sprintf(buf, "%8phd\n", ns->eui); > + return sprintf(buf, "%8phD\n", ns->eui); > } > static DEVICE_ATTR(eui, S_IRUGO, eui_show, NULL); This looks correct. I wonder what the old code printed - does someone have a device with an EUI-64 at hand to quickly cross check what we did before?
On Fri, Nov 03, 2017 at 01:55:16PM +0100, Christoph Hellwig wrote: > On Fri, Nov 03, 2017 at 11:02:50AM +0100, Javier González wrote: > > Signed-off-by: Javier González <javier@cnexlabs.com> > > --- > > drivers/nvme/host/core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > > index ae8ab0a1ef0d..f05c81774abf 100644 > > --- a/drivers/nvme/host/core.c > > +++ b/drivers/nvme/host/core.c > > @@ -2108,7 +2108,7 @@ static ssize_t eui_show(struct device *dev, struct device_attribute *attr, > > char *buf) > > { > > struct nvme_ns *ns = nvme_get_ns_from_dev(dev); > > - return sprintf(buf, "%8phd\n", ns->eui); > > + return sprintf(buf, "%8phD\n", ns->eui); > > } > > static DEVICE_ATTR(eui, S_IRUGO, eui_show, NULL); > > This looks correct. I wonder what the old code printed - does someone > have a device with an EUI-64 at hand to quickly cross check what we > did before? It just prints the same as the 'ph' format, which would look like this: 01 02 03 04 05 06 07 08 The change will make it look like this: 01-02-03-04-05-06-07-08 I think that was the original intention. Reviewed-by: Keith Busch <keith.busch@intel.com>
On Fri, 2017-11-03 at 13:55 +0100, Christoph Hellwig wrote: > On Fri, Nov 03, 2017 at 11:02:50AM +0100, Javier González wrote: > > Signed-off-by: Javier González <javier@cnexlabs.com> [] > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c [] > > @@ -2108,7 +2108,7 @@ static ssize_t eui_show(struct device *dev, struct device_attribute *attr, > > char *buf) > > { > > struct nvme_ns *ns = nvme_get_ns_from_dev(dev); > > - return sprintf(buf, "%8phd\n", ns->eui); > > + return sprintf(buf, "%8phD\n", ns->eui); > > } > > static DEVICE_ATTR(eui, S_IRUGO, eui_show, NULL); > > This looks correct. I wonder what the old code printed - does someone > have a device with an EUI-64 at hand to quickly cross check what we > did before? It uses spaces between bytes and not dashes. The code has been this way a couple years now. I think this proposal, while it might fix an unintentional output style, could also be an API and could cause user breakage if changed. Perhaps this should just become %8ph without D
> On 3 Nov 2017, at 16.16, Joe Perches <joe@perches.com> wrote: > > On Fri, 2017-11-03 at 13:55 +0100, Christoph Hellwig wrote: >> On Fri, Nov 03, 2017 at 11:02:50AM +0100, Javier González wrote: >>> Signed-off-by: Javier González <javier@cnexlabs.com> > [] >>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > [] >>> @@ -2108,7 +2108,7 @@ static ssize_t eui_show(struct device *dev, struct device_attribute *attr, >>> char *buf) >>> { >>> struct nvme_ns *ns = nvme_get_ns_from_dev(dev); >>> - return sprintf(buf, "%8phd\n", ns->eui); >>> + return sprintf(buf, "%8phD\n", ns->eui); >>> } >>> static DEVICE_ATTR(eui, S_IRUGO, eui_show, NULL); >> >> This looks correct. I wonder what the old code printed - does someone >> have a device with an EUI-64 at hand to quickly cross check what we >> did before? > > It uses spaces between bytes and not dashes. > > The code has been this way a couple years now. > > I think this proposal, while it might fix an > unintentional output style, could also be an API > and could cause user breakage if changed. > > Perhaps this should just become > > %8ph > > without D That would be ok with me. Javier.
On Sat, Nov 04, 2017 at 12:22:20PM +0100, Javier González wrote: > > Perhaps this should just become > > > > %8ph > > > > without D > > That would be ok with me. Can you resend a patch for that?
> On 7 Nov 2017, at 17.28, Christoph Hellwig <hch@lst.de> wrote: > > On Sat, Nov 04, 2017 at 12:22:20PM +0100, Javier González wrote: >>> Perhaps this should just become >>> >>> %8ph >>> >>> without D >> >> That would be ok with me. > > Can you resend a patch for that? > Sure. I’ll send it tomorrow together with the copy fix with the right commit message. Javier
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index ae8ab0a1ef0d..f05c81774abf 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2108,7 +2108,7 @@ static ssize_t eui_show(struct device *dev, struct device_attribute *attr, char *buf) { struct nvme_ns *ns = nvme_get_ns_from_dev(dev); - return sprintf(buf, "%8phd\n", ns->eui); + return sprintf(buf, "%8phD\n", ns->eui); } static DEVICE_ATTR(eui, S_IRUGO, eui_show, NULL);
Signed-off-by: Javier González <javier@cnexlabs.com> --- drivers/nvme/host/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)