* [PATCH] mount: avoid po_destroy to modify errno what we really want @ 2010-11-25 10:07 Mi Jinlong 2010-11-29 14:45 ` Chuck Lever 0 siblings, 1 reply; 6+ messages in thread From: Mi Jinlong @ 2010-11-25 10:07 UTC (permalink / raw) To: Steve Dickson; +Cc: NFSv3 list We should return the errno that was set before po_destroy, rather than the errno that was set at po_destroy. Because the po_destroy function don't affect the return value, this patch just revert the saved errno after po_destroy. Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> --- utils/mount/stropts.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c index 50a1a2a..d554877 100644 --- a/utils/mount/stropts.c +++ b/utils/mount/stropts.c @@ -592,7 +592,7 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, struct sockaddr *sap, socklen_t salen) { struct mount_options *options = po_dup(mi->options); - int result = 0; + int result = 0, save = 0; if (!options) { errno = ENOMEM; @@ -637,7 +637,9 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, result = nfs_sys_mount(mi, options); out_fail: + save = errno; po_destroy(options); + errno = save; return result; } @@ -673,7 +675,7 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, struct sockaddr *sap, socklen_t salen) { struct mount_options *options = po_dup(mi->options); - int result = 0; + int result = 0, save = 0; if (!options) { errno = ENOMEM; @@ -724,7 +726,9 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, result = nfs_sys_mount(mi, options); out_fail: + save = errno; po_destroy(options); + errno = save; return result; } -- 1.7.3.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mount: avoid po_destroy to modify errno what we really want 2010-11-25 10:07 [PATCH] mount: avoid po_destroy to modify errno what we really want Mi Jinlong @ 2010-11-29 14:45 ` Chuck Lever 2010-11-29 15:20 ` Steve Dickson 0 siblings, 1 reply; 6+ messages in thread From: Chuck Lever @ 2010-11-29 14:45 UTC (permalink / raw) To: Mi Jinlong; +Cc: Steve Dickson, NFSv3 list On Nov 25, 2010, at 5:07 AM, Mi Jinlong wrote: > We should return the errno that was set before po_destroy, > rather than the errno that was set at po_destroy. > > Because the po_destroy function don't affect the return value, > this patch just revert the saved errno after po_destroy. The only library function used in po_destroy() is free(3). Does free(3) change the value of errno? > Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com> > Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> > --- > utils/mount/stropts.c | 8 ++++++-- > 1 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c > index 50a1a2a..d554877 100644 > --- a/utils/mount/stropts.c > +++ b/utils/mount/stropts.c > @@ -592,7 +592,7 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, > struct sockaddr *sap, socklen_t salen) > { > struct mount_options *options = po_dup(mi->options); > - int result = 0; > + int result = 0, save = 0; > > if (!options) { > errno = ENOMEM; > @@ -637,7 +637,9 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, > result = nfs_sys_mount(mi, options); > > out_fail: > + save = errno; > po_destroy(options); > + errno = save; > return result; > } > > @@ -673,7 +675,7 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, > struct sockaddr *sap, socklen_t salen) > { > struct mount_options *options = po_dup(mi->options); > - int result = 0; > + int result = 0, save = 0; > > if (!options) { > errno = ENOMEM; > @@ -724,7 +726,9 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, > result = nfs_sys_mount(mi, options); > > out_fail: > + save = errno; > po_destroy(options); > + errno = save; > return result; > } > > -- > 1.7.3.2 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever chuck[dot]lever[at]oracle[dot]com ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mount: avoid po_destroy to modify errno what we really want 2010-11-29 14:45 ` Chuck Lever @ 2010-11-29 15:20 ` Steve Dickson 2010-11-30 2:05 ` Bian Naimeng 0 siblings, 1 reply; 6+ messages in thread From: Steve Dickson @ 2010-11-29 15:20 UTC (permalink / raw) To: Chuck Lever; +Cc: Mi Jinlong, NFSv3 list On 11/29/2010 09:45 AM, Chuck Lever wrote: > > On Nov 25, 2010, at 5:07 AM, Mi Jinlong wrote: > >> We should return the errno that was set before po_destroy, >> rather than the errno that was set at po_destroy. >> >> Because the po_destroy function don't affect the return value, >> this patch just revert the saved errno after po_destroy. > > The only library function used in po_destroy() is free(3). > Does free(3) change the value of errno? Looking at the man page and taking a look a the glibc code it appears the answer is no. free(3) does not set errno. Bian, what was the problem you were seeing that this patch fixed? steved. > >> Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com> >> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> >> --- >> utils/mount/stropts.c | 8 ++++++-- >> 1 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c >> index 50a1a2a..d554877 100644 >> --- a/utils/mount/stropts.c >> +++ b/utils/mount/stropts.c >> @@ -592,7 +592,7 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, >> struct sockaddr *sap, socklen_t salen) >> { >> struct mount_options *options = po_dup(mi->options); >> - int result = 0; >> + int result = 0, save = 0; >> >> if (!options) { >> errno = ENOMEM; >> @@ -637,7 +637,9 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, >> result = nfs_sys_mount(mi, options); >> >> out_fail: >> + save = errno; >> po_destroy(options); >> + errno = save; >> return result; >> } >> >> @@ -673,7 +675,7 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, >> struct sockaddr *sap, socklen_t salen) >> { >> struct mount_options *options = po_dup(mi->options); >> - int result = 0; >> + int result = 0, save = 0; >> >> if (!options) { >> errno = ENOMEM; >> @@ -724,7 +726,9 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, >> result = nfs_sys_mount(mi, options); >> >> out_fail: >> + save = errno; >> po_destroy(options); >> + errno = save; >> return result; >> } >> >> -- >> 1.7.3.2 >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mount: avoid po_destroy to modify errno what we really want 2010-11-29 15:20 ` Steve Dickson @ 2010-11-30 2:05 ` Bian Naimeng 2010-11-30 15:11 ` Chuck Lever 0 siblings, 1 reply; 6+ messages in thread From: Bian Naimeng @ 2010-11-30 2:05 UTC (permalink / raw) To: Steve Dickson; +Cc: Chuck Lever, Mi Jinlong, NFSv3 list Steve Dickson wrote: > > On 11/29/2010 09:45 AM, Chuck Lever wrote: >> On Nov 25, 2010, at 5:07 AM, Mi Jinlong wrote: >> >>> We should return the errno that was set before po_destroy, >>> rather than the errno that was set at po_destroy. >>> >>> Because the po_destroy function don't affect the return value, >>> this patch just revert the saved errno after po_destroy. >> The only library function used in po_destroy() is free(3). >> Does free(3) change the value of errno? > Looking at the man page and taking a look a the > glibc code it appears the answer is no. free(3) > does not set errno. > > Bian, what was the problem you were seeing that this > patch fixed? > Actually, there's no problem now. But this's not a good style, po_destroy maybe change the errno later on such as adding a printf. So if somebady want modify the po_destroy, he must be careful of it. :) And, i don't think it's a good idea that explicitly set errno, it maybe make some trouble in the future. It's better returning error in nfs_do_mount_v3v2, or save the error in a input parameter. :) Thanks Bian > steved. > >>> Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com> >>> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> >>> --- >>> utils/mount/stropts.c | 8 ++++++-- >>> 1 files changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c >>> index 50a1a2a..d554877 100644 >>> --- a/utils/mount/stropts.c >>> +++ b/utils/mount/stropts.c >>> @@ -592,7 +592,7 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, >>> struct sockaddr *sap, socklen_t salen) >>> { >>> struct mount_options *options = po_dup(mi->options); >>> - int result = 0; >>> + int result = 0, save = 0; >>> >>> if (!options) { >>> errno = ENOMEM; >>> @@ -637,7 +637,9 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, >>> result = nfs_sys_mount(mi, options); >>> >>> out_fail: >>> + save = errno; >>> po_destroy(options); >>> + errno = save; >>> return result; >>> } >>> >>> @@ -673,7 +675,7 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, >>> struct sockaddr *sap, socklen_t salen) >>> { >>> struct mount_options *options = po_dup(mi->options); >>> - int result = 0; >>> + int result = 0, save = 0; >>> >>> if (!options) { >>> errno = ENOMEM; >>> @@ -724,7 +726,9 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, >>> result = nfs_sys_mount(mi, options); >>> >>> out_fail: >>> + save = errno; >>> po_destroy(options); >>> + errno = save; >>> return result; >>> } >>> >>> -- >>> 1.7.3.2 >>> >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- Regards Bian Naimeng ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mount: avoid po_destroy to modify errno what we really want 2010-11-30 2:05 ` Bian Naimeng @ 2010-11-30 15:11 ` Chuck Lever 2010-12-01 1:01 ` Bian Naimeng 0 siblings, 1 reply; 6+ messages in thread From: Chuck Lever @ 2010-11-30 15:11 UTC (permalink / raw) To: Bian Naimeng; +Cc: Steve Dickson, Mi Jinlong, NFSv3 list Hi Bian On Nov 29, 2010, at 9:05 PM, Bian Naimeng wrote: > > > Steve Dickson wrote: >> >> On 11/29/2010 09:45 AM, Chuck Lever wrote: >>> On Nov 25, 2010, at 5:07 AM, Mi Jinlong wrote: >>> >>>> We should return the errno that was set before po_destroy, >>>> rather than the errno that was set at po_destroy. >>>> >>>> Because the po_destroy function don't affect the return value, >>>> this patch just revert the saved errno after po_destroy. >>> The only library function used in po_destroy() is free(3). >>> Does free(3) change the value of errno? >> Looking at the man page and taking a look a the >> glibc code it appears the answer is no. free(3) >> does not set errno. >> >> Bian, what was the problem you were seeing that this >> patch fixed? >> > > Actually, there's no problem now. But this's not a good style, po_destroy > maybe change the errno later on such as adding a printf. > So if somebady want modify the po_destroy, he must be careful of it. :) > > And, i don't think it's a good idea that explicitly set errno, it maybe make > some trouble in the future. It's better returning error in nfs_do_mount_v3v2, > or save the error in a input parameter. :) Perhaps it isn't good style, but I'd rather see all of stropts.c updated to pass errno as an explicit parameter than to have it changed piecemeal. I agree that we run into trouble everywhere in mount with errno. I think this particular patch is unnecessary until we have a need for it. Basically it confuses a reviewer who sees that we've gone to the trouble here to save and restore errno, but nowhere else, and besides po_destroy() doesn't even alter errno. Why are we bothering? > Thanks > Bian > >> steved. >> >>>> Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com> >>>> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> >>>> --- >>>> utils/mount/stropts.c | 8 ++++++-- >>>> 1 files changed, 6 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c >>>> index 50a1a2a..d554877 100644 >>>> --- a/utils/mount/stropts.c >>>> +++ b/utils/mount/stropts.c >>>> @@ -592,7 +592,7 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, >>>> struct sockaddr *sap, socklen_t salen) >>>> { >>>> struct mount_options *options = po_dup(mi->options); >>>> - int result = 0; >>>> + int result = 0, save = 0; >>>> >>>> if (!options) { >>>> errno = ENOMEM; >>>> @@ -637,7 +637,9 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, >>>> result = nfs_sys_mount(mi, options); >>>> >>>> out_fail: >>>> + save = errno; >>>> po_destroy(options); >>>> + errno = save; >>>> return result; >>>> } >>>> >>>> @@ -673,7 +675,7 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, >>>> struct sockaddr *sap, socklen_t salen) >>>> { >>>> struct mount_options *options = po_dup(mi->options); >>>> - int result = 0; >>>> + int result = 0, save = 0; >>>> >>>> if (!options) { >>>> errno = ENOMEM; >>>> @@ -724,7 +726,9 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, >>>> result = nfs_sys_mount(mi, options); >>>> >>>> out_fail: >>>> + save = errno; >>>> po_destroy(options); >>>> + errno = save; >>>> return result; >>>> } >>>> >>>> -- >>>> 1.7.3.2 >>>> >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> > > -- > Regards > Bian Naimeng > -- Chuck Lever chuck[dot]lever[at]oracle[dot]com ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mount: avoid po_destroy to modify errno what we really want 2010-11-30 15:11 ` Chuck Lever @ 2010-12-01 1:01 ` Bian Naimeng 0 siblings, 0 replies; 6+ messages in thread From: Bian Naimeng @ 2010-12-01 1:01 UTC (permalink / raw) To: Chuck Lever; +Cc: Steve Dickson, Mi Jinlong, NFSv3 list Chuck Lever wrote: > Hi Bian > > On Nov 29, 2010, at 9:05 PM, Bian Naimeng wrote: > >> >> Steve Dickson wrote: >>> On 11/29/2010 09:45 AM, Chuck Lever wrote: >>>> On Nov 25, 2010, at 5:07 AM, Mi Jinlong wrote: >>>> >>>>> We should return the errno that was set before po_destroy, >>>>> rather than the errno that was set at po_destroy. >>>>> >>>>> Because the po_destroy function don't affect the return value, >>>>> this patch just revert the saved errno after po_destroy. >>>> The only library function used in po_destroy() is free(3). >>>> Does free(3) change the value of errno? >>> Looking at the man page and taking a look a the >>> glibc code it appears the answer is no. free(3) >>> does not set errno. >>> >>> Bian, what was the problem you were seeing that this >>> patch fixed? >>> >> Actually, there's no problem now. But this's not a good style, po_destroy >> maybe change the errno later on such as adding a printf. >> So if somebady want modify the po_destroy, he must be careful of it. :) >> >> And, i don't think it's a good idea that explicitly set errno, it maybe make >> some trouble in the future. It's better returning error in nfs_do_mount_v3v2, >> or save the error in a input parameter. :) > > Perhaps it isn't good style, but I'd rather see all of stropts.c updated to pass errno as an explicit parameter than to have it changed piecemeal. I agree that we run into trouble everywhere in mount with errno. > > I think this particular patch is unnecessary until we have a need for it. Basically it confuses a reviewer who sees that we've gone to the trouble here to save and restore errno, but nowhere else, and besides po_destroy() doesn't even alter errno. Why are we bothering? > Well, it's okay to me. Thanks Bian >>> steved. >>> >>>>> Signed-off-by: Bian Naimeng <biannm@cn.fujitsu.com> >>>>> Signed-off-by: Mi Jinlong <mijinlong@cn.fujitsu.com> >>>>> --- >>>>> utils/mount/stropts.c | 8 ++++++-- >>>>> 1 files changed, 6 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c >>>>> index 50a1a2a..d554877 100644 >>>>> --- a/utils/mount/stropts.c >>>>> +++ b/utils/mount/stropts.c >>>>> @@ -592,7 +592,7 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, >>>>> struct sockaddr *sap, socklen_t salen) >>>>> { >>>>> struct mount_options *options = po_dup(mi->options); >>>>> - int result = 0; >>>>> + int result = 0, save = 0; >>>>> >>>>> if (!options) { >>>>> errno = ENOMEM; >>>>> @@ -637,7 +637,9 @@ static int nfs_do_mount_v3v2(struct nfsmount_info *mi, >>>>> result = nfs_sys_mount(mi, options); >>>>> >>>>> out_fail: >>>>> + save = errno; >>>>> po_destroy(options); >>>>> + errno = save; >>>>> return result; >>>>> } >>>>> >>>>> @@ -673,7 +675,7 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, >>>>> struct sockaddr *sap, socklen_t salen) >>>>> { >>>>> struct mount_options *options = po_dup(mi->options); >>>>> - int result = 0; >>>>> + int result = 0, save = 0; >>>>> >>>>> if (!options) { >>>>> errno = ENOMEM; >>>>> @@ -724,7 +726,9 @@ static int nfs_do_mount_v4(struct nfsmount_info *mi, >>>>> result = nfs_sys_mount(mi, options); >>>>> >>>>> out_fail: >>>>> + save = errno; >>>>> po_destroy(options); >>>>> + errno = save; >>>>> return result; >>>>> } >>>>> >>>>> -- >>>>> 1.7.3.2 >>>>> >>>>> >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>>> the body of a message to majordomo@vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >>> >> -- >> Regards >> Bian Naimeng >> > -- Regards Bian Naimeng ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-12-01 1:01 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-25 10:07 [PATCH] mount: avoid po_destroy to modify errno what we really want Mi Jinlong 2010-11-29 14:45 ` Chuck Lever 2010-11-29 15:20 ` Steve Dickson 2010-11-30 2:05 ` Bian Naimeng 2010-11-30 15:11 ` Chuck Lever 2010-12-01 1:01 ` Bian Naimeng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).