* [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value @ 2015-05-13 6:29 Wenlin Kang 2015-05-20 19:33 ` Brian Norris 0 siblings, 1 reply; 11+ messages in thread From: Wenlin Kang @ 2015-05-13 6:29 UTC (permalink / raw) To: dwmw2, computersforpeace; +Cc: linux-mtd, linux-kernel Modify function blktrans_getgeo()'s return value to -ENXIO when dev->tr->getgeo == NULL. We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, because the function blktrans_getgeo() has an output value "hd_geometry" which is usually used by some application, if return 0, it will make some application get the wrong information. Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> --- drivers/mtd/mtd_blkdevs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index 2b0c5287..f8bb16e 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) if (!dev->mtd) goto unlock; - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO; unlock: mutex_unlock(&dev->lock); blktrans_dev_put(dev); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value 2015-05-13 6:29 [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value Wenlin Kang @ 2015-05-20 19:33 ` Brian Norris [not found] ` <555CE4BF.8020506@gmail.com> 2015-05-21 7:56 ` Richard Weinberger 0 siblings, 2 replies; 11+ messages in thread From: Brian Norris @ 2015-05-20 19:33 UTC (permalink / raw) To: Wenlin Kang; +Cc: dwmw2, linux-mtd, linux-kernel Hi Wenlin, In the subject: s/rerurn/return/ On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote: > Modify function blktrans_getgeo()'s return value to -ENXIO when > dev->tr->getgeo == NULL. > > We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, > because the function blktrans_getgeo() has an output value "hd_geometry" > which is usually used by some application, if return 0, it will make some > application get the wrong information. > > Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> > --- > drivers/mtd/mtd_blkdevs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c > index 2b0c5287..f8bb16e 100644 > --- a/drivers/mtd/mtd_blkdevs.c > +++ b/drivers/mtd/mtd_blkdevs.c > @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) > if (!dev->mtd) > goto unlock; > > - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; > + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO; Good catch. I don't think ENXIO is correct in this case, though. Maybe -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess. > unlock: > mutex_unlock(&dev->lock); > blktrans_dev_put(dev); Thanks, Brian ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <555CE4BF.8020506@gmail.com>]
* Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value [not found] ` <555CE4BF.8020506@gmail.com> @ 2015-05-21 6:49 ` Wenlin Kang 2015-05-21 7:37 ` Brian Norris 0 siblings, 1 reply; 11+ messages in thread From: Wenlin Kang @ 2015-05-21 6:49 UTC (permalink / raw) To: nick, Brian Norris; +Cc: linux-mtd, dwmw2, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2084 bytes --] On 2015年05月21日 03:47, nick wrote: > > On 2015-05-20 03:33 PM, Brian Norris wrote: >> Hi Wenlin, >> >> In the subject: >> >> s/rerurn/return/ Thanks for pointing out this, I will modify it. >> >> On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote: >>> Modify function blktrans_getgeo()'s return value to -ENXIO when >>> dev->tr->getgeo == NULL. >>> >>> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, >>> because the function blktrans_getgeo() has an output value "hd_geometry" >>> which is usually used by some application, if return 0, it will make some >>> application get the wrong information. >>> >>> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> >>> --- >>> drivers/mtd/mtd_blkdevs.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c >>> index 2b0c5287..f8bb16e 100644 >>> --- a/drivers/mtd/mtd_blkdevs.c >>> +++ b/drivers/mtd/mtd_blkdevs.c >>> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) >>> if (!dev->mtd) >>> goto unlock; >>> >>> - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; >>> + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO; >> Good catch. I don't think ENXIO is correct in this case, though. Maybe >> -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess. >> > I would recommend -EOPNOTSUPP as this as nothing to do with unimplemented > functions or hardware support. This is just unsupported due to the value > being NULL and therefore the hardware support is not there. > Just My Option, > Nick As you said, -EOPNOTSUPP might be better,thanks. Hi Brian I have remade the patch and attached it, would you please check it again? thanks. >>> unlock: >>> mutex_unlock(&dev->lock); >>> blktrans_dev_put(dev); >> Thanks, >> Brian >> >> ______________________________________________________ >> Linux MTD discussion mailing list >> http://lists.infradead.org/mailman/listinfo/linux-mtd/ >> > -- Thanks, Wenlin Kang [-- Attachment #2: 0001-mtd-blktrans-change-blktrans_getgeo-return-value.patch --] [-- Type: text/x-diff, Size: 1216 bytes --] >From d8fa6589438a6992ed13b91c0124ed6cd8114eff Mon Sep 17 00:00:00 2001 From: Wenlin Kang <wenlin.kang@windriver.com> Date: Thu, 21 May 2015 14:11:58 +0800 Subject: [PATCH] mtd: blktrans: change blktrans_getgeo return value Modify function blktrans_getgeo()'s return value to -EOPNOTSUPP when dev->tr->getgeo == NULL. We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, because the function blktrans_getgeo() has an output value "hd_geometry" which is usually used by some application, if return 0, it will make some application get the wrong information. Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> --- drivers/mtd/mtd_blkdevs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index 2b0c5287..2012216 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) if (!dev->mtd) goto unlock; - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -EOPNOTSUPP; unlock: mutex_unlock(&dev->lock); blktrans_dev_put(dev); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value 2015-05-21 6:49 ` Wenlin Kang @ 2015-05-21 7:37 ` Brian Norris 2015-05-21 8:01 ` Wenlin Kang 0 siblings, 1 reply; 11+ messages in thread From: Brian Norris @ 2015-05-21 7:37 UTC (permalink / raw) To: Wenlin Kang; +Cc: linux-mtd, dwmw2, linux-kernel On Thu, May 21, 2015 at 02:49:38PM +0800, Wenlin Kang wrote: > On 2015年05月21日 03:47, nick wrote: > >On 2015-05-20 03:33 PM, Brian Norris wrote: > >>On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote: > >>>Modify function blktrans_getgeo()'s return value to -ENXIO when > >>>dev->tr->getgeo == NULL. > >>> > >>>We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, > >>>because the function blktrans_getgeo() has an output value "hd_geometry" > >>>which is usually used by some application, if return 0, it will make some > >>>application get the wrong information. > >>> > >>>Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> > >>>--- > >>> drivers/mtd/mtd_blkdevs.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>>diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c > >>>index 2b0c5287..f8bb16e 100644 > >>>--- a/drivers/mtd/mtd_blkdevs.c > >>>+++ b/drivers/mtd/mtd_blkdevs.c > >>>@@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) > >>> if (!dev->mtd) > >>> goto unlock; > >>>- ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; > >>>+ ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO; > >>Good catch. I don't think ENXIO is correct in this case, though. Maybe > >>-EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess. > >> > >I would recommend -EOPNOTSUPP as this as nothing to do with unimplemented > >functions or hardware support. This is just unsupported due to the value > >being NULL and therefore the hardware support is not there. > >Just My Option, > >Nick > > As you said, -EOPNOTSUPP might be better,thanks. Nick's opinion is irrelevant here: https://lkml.org/lkml/2014/8/4/206 > I have remade the patch and attached it, would you please check it > again? thanks. Please send patches inline (e.g., via git-send-email), not as attachments. But this is pretty trivial, and it's what I already tested. So, applied to l2-mtd.git. Brian ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value 2015-05-21 7:37 ` Brian Norris @ 2015-05-21 8:01 ` Wenlin Kang 0 siblings, 0 replies; 11+ messages in thread From: Wenlin Kang @ 2015-05-21 8:01 UTC (permalink / raw) To: Brian Norris; +Cc: linux-mtd, dwmw2, linux-kernel On 2015年05月21日 15:37, Brian Norris wrote: > On Thu, May 21, 2015 at 02:49:38PM +0800, Wenlin Kang wrote: >> On 2015年05月21日 03:47, nick wrote: >>> On 2015-05-20 03:33 PM, Brian Norris wrote: >>>> On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote: >>>>> Modify function blktrans_getgeo()'s return value to -ENXIO when >>>>> dev->tr->getgeo == NULL. >>>>> >>>>> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, >>>>> because the function blktrans_getgeo() has an output value "hd_geometry" >>>>> which is usually used by some application, if return 0, it will make some >>>>> application get the wrong information. >>>>> >>>>> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> >>>>> --- >>>>> drivers/mtd/mtd_blkdevs.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c >>>>> index 2b0c5287..f8bb16e 100644 >>>>> --- a/drivers/mtd/mtd_blkdevs.c >>>>> +++ b/drivers/mtd/mtd_blkdevs.c >>>>> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) >>>>> if (!dev->mtd) >>>>> goto unlock; >>>>> - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; >>>>> + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO; >>>> Good catch. I don't think ENXIO is correct in this case, though. Maybe >>>> -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess. >>>> >>> I would recommend -EOPNOTSUPP as this as nothing to do with unimplemented >>> functions or hardware support. This is just unsupported due to the value >>> being NULL and therefore the hardware support is not there. >>> Just My Option, >>> Nick >> As you said, -EOPNOTSUPP might be better,thanks. > Nick's opinion is irrelevant here: > > https://lkml.org/lkml/2014/8/4/206 > >> I have remade the patch and attached it, would you please check it >> again? thanks. > Please send patches inline (e.g., via git-send-email), not as > attachments. But this is pretty trivial, and it's what I already tested. > > So, applied to l2-mtd.git. > > Brian > OK, thanks, I have resent the patch to you and list by git send-email, please check it. -- Thanks, Wenlin Kang ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value 2015-05-20 19:33 ` Brian Norris [not found] ` <555CE4BF.8020506@gmail.com> @ 2015-05-21 7:56 ` Richard Weinberger 2015-05-21 8:05 ` Brian Norris 1 sibling, 1 reply; 11+ messages in thread From: Richard Weinberger @ 2015-05-21 7:56 UTC (permalink / raw) To: Brian Norris Cc: Wenlin Kang, linux-mtd@lists.infradead.org, David Woodhouse, LKML On Wed, May 20, 2015 at 9:33 PM, Brian Norris <computersforpeace@gmail.com> wrote: > Hi Wenlin, > > In the subject: > > s/rerurn/return/ > > On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote: >> Modify function blktrans_getgeo()'s return value to -ENXIO when >> dev->tr->getgeo == NULL. >> >> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, >> because the function blktrans_getgeo() has an output value "hd_geometry" >> which is usually used by some application, if return 0, it will make some >> application get the wrong information. >> >> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> >> --- >> drivers/mtd/mtd_blkdevs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c >> index 2b0c5287..f8bb16e 100644 >> --- a/drivers/mtd/mtd_blkdevs.c >> +++ b/drivers/mtd/mtd_blkdevs.c >> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) >> if (!dev->mtd) >> goto unlock; >> >> - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; >> + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO; > > Good catch. I don't think ENXIO is correct in this case, though. Maybe > -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess. I'd vote for -ENOTTY as this is what HDIO_GETGEIO returns if the function is not implemented and blktrans_getgeo() is only a wrapper around that. See https://www.kernel.org/doc/Documentation/ioctl/hdio.txt and block/ioctl.c. -- Thanks, //richard ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value 2015-05-21 7:56 ` Richard Weinberger @ 2015-05-21 8:05 ` Brian Norris 2015-05-21 8:22 ` Wenlin Kang 2015-05-21 10:17 ` Richard Weinberger 0 siblings, 2 replies; 11+ messages in thread From: Brian Norris @ 2015-05-21 8:05 UTC (permalink / raw) To: Richard Weinberger Cc: Wenlin Kang, linux-mtd@lists.infradead.org, David Woodhouse, LKML On Thu, May 21, 2015 at 09:56:21AM +0200, Richard Weinberger wrote: > On Wed, May 20, 2015 at 9:33 PM, Brian Norris > <computersforpeace@gmail.com> wrote: > > On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote: > >> Modify function blktrans_getgeo()'s return value to -ENXIO when > >> dev->tr->getgeo == NULL. > >> > >> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, > >> because the function blktrans_getgeo() has an output value "hd_geometry" > >> which is usually used by some application, if return 0, it will make some > >> application get the wrong information. > >> > >> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> > >> --- > >> drivers/mtd/mtd_blkdevs.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c > >> index 2b0c5287..f8bb16e 100644 > >> --- a/drivers/mtd/mtd_blkdevs.c > >> +++ b/drivers/mtd/mtd_blkdevs.c > >> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) > >> if (!dev->mtd) > >> goto unlock; > >> > >> - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; > >> + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO; > > > > Good catch. I don't think ENXIO is correct in this case, though. Maybe > > -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess. > > I'd vote for -ENOTTY as this is what HDIO_GETGEIO returns > if the function is not implemented and blktrans_getgeo() > is only a wrapper around that. I suppose that makes more sense. > See https://www.kernel.org/doc/Documentation/ioctl/hdio.txt But this only mentions EINVAL... > and block/ioctl.c. Anyway, I pushed EOPNOTSUPP, but I can fix that up if we agree. Brian ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value 2015-05-21 8:05 ` Brian Norris @ 2015-05-21 8:22 ` Wenlin Kang 2015-05-21 10:17 ` Richard Weinberger 1 sibling, 0 replies; 11+ messages in thread From: Wenlin Kang @ 2015-05-21 8:22 UTC (permalink / raw) To: Brian Norris, Richard Weinberger Cc: linux-mtd@lists.infradead.org, David Woodhouse, LKML On 2015年05月21日 16:05, Brian Norris wrote: > On Thu, May 21, 2015 at 09:56:21AM +0200, Richard Weinberger wrote: >> On Wed, May 20, 2015 at 9:33 PM, Brian Norris >> <computersforpeace@gmail.com> wrote: >>> On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote: >>>> Modify function blktrans_getgeo()'s return value to -ENXIO when >>>> dev->tr->getgeo == NULL. >>>> >>>> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, >>>> because the function blktrans_getgeo() has an output value "hd_geometry" >>>> which is usually used by some application, if return 0, it will make some >>>> application get the wrong information. >>>> >>>> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> >>>> --- >>>> drivers/mtd/mtd_blkdevs.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c >>>> index 2b0c5287..f8bb16e 100644 >>>> --- a/drivers/mtd/mtd_blkdevs.c >>>> +++ b/drivers/mtd/mtd_blkdevs.c >>>> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) >>>> if (!dev->mtd) >>>> goto unlock; >>>> >>>> - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; >>>> + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO; >>> Good catch. I don't think ENXIO is correct in this case, though. Maybe >>> -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess. >> I'd vote for -ENOTTY as this is what HDIO_GETGEIO returns >> if the function is not implemented and blktrans_getgeo() >> is only a wrapper around that. > I suppose that makes more sense. > >> See https://www.kernel.org/doc/Documentation/ioctl/hdio.txt > But this only mentions EINVAL... > >> and block/ioctl.c. > Anyway, I pushed EOPNOTSUPP, but I can fix that up if we agree. > > Brian > > Hi Brian I just read block/compat_ioctl.c, in that I found the follow, 51 static int compat_hdio_getgeo(struct gendisk *disk, struct block_device *bdev, 52 struct compat_hd_geometry __user *ugeo) 53 { 54 struct hd_geometry geo; 55 int ret; 56 57 if (!ugeo) 58 return -EINVAL; 59 if (!disk->fops->getgeo) 60 return -ENOTTY; So, as Richard said, -ENOTTY should also be OK, and before last commit(048d87199566663e4edc4880df3703c04bcf41d9), it's -ENOTTY, so I think you can modify it, or let know whether I need to resent it again. -- Thanks, Wenlin Kang ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value 2015-05-21 8:05 ` Brian Norris 2015-05-21 8:22 ` Wenlin Kang @ 2015-05-21 10:17 ` Richard Weinberger 2015-05-21 17:54 ` [PATCH] mtd: blktrans: use better error code for unimplemented ioctl() Brian Norris 1 sibling, 1 reply; 11+ messages in thread From: Richard Weinberger @ 2015-05-21 10:17 UTC (permalink / raw) To: Brian Norris Cc: Wenlin Kang, linux-mtd@lists.infradead.org, David Woodhouse, LKML Am 21.05.2015 um 10:05 schrieb Brian Norris: > On Thu, May 21, 2015 at 09:56:21AM +0200, Richard Weinberger wrote: >> On Wed, May 20, 2015 at 9:33 PM, Brian Norris >> <computersforpeace@gmail.com> wrote: >>> On Wed, May 13, 2015 at 02:29:16PM +0800, Wenlin Kang wrote: >>>> Modify function blktrans_getgeo()'s return value to -ENXIO when >>>> dev->tr->getgeo == NULL. >>>> >>>> We shouldn't make the return value to 0 when dev->tr->getgeo == NULL, >>>> because the function blktrans_getgeo() has an output value "hd_geometry" >>>> which is usually used by some application, if return 0, it will make some >>>> application get the wrong information. >>>> >>>> Signed-off-by: Wenlin Kang <wenlin.kang@windriver.com> >>>> --- >>>> drivers/mtd/mtd_blkdevs.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c >>>> index 2b0c5287..f8bb16e 100644 >>>> --- a/drivers/mtd/mtd_blkdevs.c >>>> +++ b/drivers/mtd/mtd_blkdevs.c >>>> @@ -273,7 +273,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) >>>> if (!dev->mtd) >>>> goto unlock; >>>> >>>> - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : 0; >>>> + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENXIO; >>> >>> Good catch. I don't think ENXIO is correct in this case, though. Maybe >>> -EOPNOTSUPP or -ENOSYS? The latter might make more sense I guess. >> >> I'd vote for -ENOTTY as this is what HDIO_GETGEIO returns >> if the function is not implemented and blktrans_getgeo() >> is only a wrapper around that. > > I suppose that makes more sense. > >> See https://www.kernel.org/doc/Documentation/ioctl/hdio.txt > > But this only mentions EINVAL... Because ENOTTY is covered by ioctl(). If you request a non-existing function this error code will be returned. Thanks, //richard ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] mtd: blktrans: use better error code for unimplemented ioctl() 2015-05-21 10:17 ` Richard Weinberger @ 2015-05-21 17:54 ` Brian Norris 2015-05-27 19:15 ` Brian Norris 0 siblings, 1 reply; 11+ messages in thread From: Brian Norris @ 2015-05-21 17:54 UTC (permalink / raw) To: Richard Weinberger Cc: Wenlin Kang, linux-mtd@lists.infradead.org, David Woodhouse, LKML In commit 50183936254b ("mtd: blktrans: change blktrans_getgeo return value") we fixed the problem that ioctl(HDIO_GETGEO) might return 0 (success) for mtdblock devices which did not implement the feature and would leave a blank (zero) result. But now, let's get the error code right. Other code paths on this ioctl tend to use -ENOTTY to notify the user that the ioctl() is not supported for the device, so let's use that instead of -EOPNOTSUPP. Suggested-by: Richard Weinberger <richard@nod.at> Signed-off-by: Brian Norris <computersforpeace@gmail.com> --- Against l2-mtd.git (linux-next) drivers/mtd/mtd_blkdevs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c index c545c9325acf..41acc507b22e 100644 --- a/drivers/mtd/mtd_blkdevs.c +++ b/drivers/mtd/mtd_blkdevs.c @@ -278,7 +278,7 @@ static int blktrans_getgeo(struct block_device *bdev, struct hd_geometry *geo) if (!dev->mtd) goto unlock; - ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -EOPNOTSUPP; + ret = dev->tr->getgeo ? dev->tr->getgeo(dev, geo) : -ENOTTY; unlock: mutex_unlock(&dev->lock); blktrans_dev_put(dev); -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] mtd: blktrans: use better error code for unimplemented ioctl() 2015-05-21 17:54 ` [PATCH] mtd: blktrans: use better error code for unimplemented ioctl() Brian Norris @ 2015-05-27 19:15 ` Brian Norris 0 siblings, 0 replies; 11+ messages in thread From: Brian Norris @ 2015-05-27 19:15 UTC (permalink / raw) To: Richard Weinberger Cc: Wenlin Kang, linux-mtd@lists.infradead.org, David Woodhouse, LKML On Thu, May 21, 2015 at 10:54:31AM -0700, Brian Norris wrote: > In commit 50183936254b ("mtd: blktrans: change blktrans_getgeo return > value") we fixed the problem that ioctl(HDIO_GETGEO) might return 0 > (success) for mtdblock devices which did not implement the feature and > would leave a blank (zero) result. > > But now, let's get the error code right. Other code paths on this ioctl > tend to use -ENOTTY to notify the user that the ioctl() is not supported > for the device, so let's use that instead of -EOPNOTSUPP. > > Suggested-by: Richard Weinberger <richard@nod.at> > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > --- > Against l2-mtd.git (linux-next) Applied. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-05-27 19:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-13 6:29 [PATCH] mtd: blktrans: change blktrans_getgeo rerurn value Wenlin Kang
2015-05-20 19:33 ` Brian Norris
[not found] ` <555CE4BF.8020506@gmail.com>
2015-05-21 6:49 ` Wenlin Kang
2015-05-21 7:37 ` Brian Norris
2015-05-21 8:01 ` Wenlin Kang
2015-05-21 7:56 ` Richard Weinberger
2015-05-21 8:05 ` Brian Norris
2015-05-21 8:22 ` Wenlin Kang
2015-05-21 10:17 ` Richard Weinberger
2015-05-21 17:54 ` [PATCH] mtd: blktrans: use better error code for unimplemented ioctl() Brian Norris
2015-05-27 19:15 ` Brian Norris
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox