| Message ID | 1351980150-24145-9-git-send-email-lee.jones@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote: > This patch fixes: > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’: > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable] Did you have CONFIG_NETDEVICES not set in this build? Paul Bolle -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 03 Nov 2012, Paul Bolle wrote: > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote: > > This patch fixes: > > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’: > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable] > > Did you have CONFIG_NETDEVICES not set in this build? Ah yes, I see it. The function went down further than I thought it did. So the real fix is to ensure 's' is defined inside of some ifdef CONFIG_NETDEVICES guards. I'll fix up and resend. Will likely be tomorrow now, as it's fast approaching midnight here.
On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote: > On Sat, 03 Nov 2012, Paul Bolle wrote: > > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote: > > > This patch fixes: > > > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’: > > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable] > > > > Did you have CONFIG_NETDEVICES not set in this build? > > Ah yes, I see it. The function went down further than I thought > it did. So the real fix is to ensure 's' is defined inside of > some ifdef CONFIG_NETDEVICES guards. What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES" guards in this file and not in isdn_net.c, were all the ioctl commands guarded that way seem to be calling into. On first glance that doesn't make much sense. (Actually the idea of having ISDN without NETDEVICES is a bit puzzling too. But there are too many parts of the isdn subsystem that I'm unfamiliar with to say whether that can make sense.) Paul Bolle -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 04 Nov 2012, Paul Bolle wrote: > On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote: > > On Sat, 03 Nov 2012, Paul Bolle wrote: > > > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote: > > > > This patch fixes: > > > > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’: > > > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable] > > > > > > Did you have CONFIG_NETDEVICES not set in this build? > > > > Ah yes, I see it. The function went down further than I thought > > it did. So the real fix is to ensure 's' is defined inside of > > some ifdef CONFIG_NETDEVICES guards. > > What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES" > guards in this file and not in isdn_net.c, were all the ioctl commands > guarded that way seem to be calling into. On first glance that doesn't > make much sense. > > (Actually the idea of having ISDN without NETDEVICES is a bit puzzling > too. But there are too many parts of the isdn subsystem that I'm > unfamiliar with to say whether that can make sense.) I'm in the same position as you Paul. I just noticed the warning so fixed it following the current way of doing things. Any, more substantial changes requiring greater knowledge of the subsystem would have to be done by someone else.
On Sun, 04 Nov 2012, David Miller wrote: > From: Lee Jones <lee.jones@linaro.org> > Date: Sun, 4 Nov 2012 11:53:32 +0100 > > > On Sun, 04 Nov 2012, Paul Bolle wrote: > > > >> On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote: > >> > On Sat, 03 Nov 2012, Paul Bolle wrote: > >> > > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote: > >> > > > This patch fixes: > >> > > > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’: > >> > > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable] > >> > > > >> > > Did you have CONFIG_NETDEVICES not set in this build? > >> > > >> > Ah yes, I see it. The function went down further than I thought > >> > it did. So the real fix is to ensure 's' is defined inside of > >> > some ifdef CONFIG_NETDEVICES guards. > >> > >> What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES" > >> guards in this file and not in isdn_net.c, were all the ioctl commands > >> guarded that way seem to be calling into. On first glance that doesn't > >> make much sense. > >> > >> (Actually the idea of having ISDN without NETDEVICES is a bit puzzling > >> too. But there are too many parts of the isdn subsystem that I'm > >> unfamiliar with to say whether that can make sense.) > > > > I'm in the same position as you Paul. I just noticed the warning so > > fixed it following the current way of doing things. Any, more > > substantial changes requiring greater knowledge of the subsystem > > would have to be done by someone else. > > I think the most appropriate thing to do is make CONFIG_ISDN depend > upon CONFIG_NETDEVICES in the Kconfig file. ... and then remove the CONFIG_NETDEVICES guards in isdn_common.c? If that's suitable more suitable I can do that instead? Just need a yay or nay and I'll make it happen.
On Mon, 2012-11-05 at 09:44 +0100, Lee Jones wrote: > On Sun, 04 Nov 2012, David Miller wrote: > > I think the most appropriate thing to do is make CONFIG_ISDN depend > > upon CONFIG_NETDEVICES in the Kconfig file. > > ... and then remove the CONFIG_NETDEVICES guards in isdn_common.c? If ISDN would depend on NETDEVICES, ISDN_I4L would too, since it depends on ISDN. In that case CONFIG_NETDEVICES would always be true when compiling isdn_common.c. That would make these guards pointless. (The dependency of ISDN_PPP on NETDEVICES would then also be pointless.) Paul Bolle -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 05 Nov 2012, Paul Bolle wrote: > On Mon, 2012-11-05 at 09:44 +0100, Lee Jones wrote: > > On Sun, 04 Nov 2012, David Miller wrote: > > > I think the most appropriate thing to do is make CONFIG_ISDN depend > > > upon CONFIG_NETDEVICES in the Kconfig file. > > > > ... and then remove the CONFIG_NETDEVICES guards in isdn_common.c? > > If ISDN would depend on NETDEVICES, ISDN_I4L would too, since it depends > on ISDN. In that case CONFIG_NETDEVICES would always be true when > compiling isdn_common.c. That would make these guards pointless. (The > dependency of ISDN_PPP on NETDEVICES would then also be pointless.) I'll submit a patch based on your comments. Thanks for your time Paul.
From: Lee Jones <lee.jones@linaro.org> Date: Mon, 5 Nov 2012 09:44:19 +0100 > On Sun, 04 Nov 2012, David Miller wrote: > >> From: Lee Jones <lee.jones@linaro.org> >> Date: Sun, 4 Nov 2012 11:53:32 +0100 >> >> > On Sun, 04 Nov 2012, Paul Bolle wrote: >> > >> >> On Sat, 2012-11-03 at 23:48 +0100, Lee Jones wrote: >> >> > On Sat, 03 Nov 2012, Paul Bolle wrote: >> >> > > On Sat, 2012-11-03 at 23:02 +0100, Lee Jones wrote: >> >> > > > This patch fixes: >> >> > > > drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’: >> >> > > > drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable] >> >> > > >> >> > > Did you have CONFIG_NETDEVICES not set in this build? >> >> > >> >> > Ah yes, I see it. The function went down further than I thought >> >> > it did. So the real fix is to ensure 's' is defined inside of >> >> > some ifdef CONFIG_NETDEVICES guards. >> >> >> >> What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES" >> >> guards in this file and not in isdn_net.c, were all the ioctl commands >> >> guarded that way seem to be calling into. On first glance that doesn't >> >> make much sense. >> >> >> >> (Actually the idea of having ISDN without NETDEVICES is a bit puzzling >> >> too. But there are too many parts of the isdn subsystem that I'm >> >> unfamiliar with to say whether that can make sense.) >> > >> > I'm in the same position as you Paul. I just noticed the warning so >> > fixed it following the current way of doing things. Any, more >> > substantial changes requiring greater knowledge of the subsystem >> > would have to be done by someone else. >> >> I think the most appropriate thing to do is make CONFIG_ISDN depend >> upon CONFIG_NETDEVICES in the Kconfig file. > > ... and then remove the CONFIG_NETDEVICES guards in isdn_common.c? > > If that's suitable more suitable I can do that instead? > > Just need a yay or nay and I'll make it happen. "yay"
diff --git a/drivers/isdn/i4l/isdn_common.c b/drivers/isdn/i4l/isdn_common.c index 8c610fa..fb90d40 100644 --- a/drivers/isdn/i4l/isdn_common.c +++ b/drivers/isdn/i4l/isdn_common.c @@ -1275,7 +1275,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg) int ret; int i; char __user *p; - char *s; union iocpar { char name[10]; char bname[22];
This patch fixes: drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’: drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable] Cc: Karsten Keil <isdn@linux-pingi.de> Cc: netdev@vger.kernel.org Signed-off-by: Lee Jones <lee.jones@linaro.org> --- drivers/isdn/i4l/isdn_common.c | 1 - 1 file changed, 1 deletion(-)