CC: LKML
On Tue, 2009-01-20 at 20:23 +0530, Jaswinder Singh Rajput wrote:
> For if_frad.h I am getting two warnings with 'make headers_check':
> usr/include/linux/if_frad.h:29: leaks CONFIG_DLCI to userspace where it is not valid
> usr/include/linux/if_frad.h:129: leaks CONFIG_DLCI to userspace where it is not valid
>
> Almost all if_frad.h is covered with #if defined(CONFIG_DLCI) || defined(CONFIG_DLCI_MODULE)
>
> If I remove this portion so only this what we get in userspace:
>
> #ifndef _FRAD_H_
> #define _FRAD_H_
>
> #include <linux/if.h>
>
> #endif
>
> So I am surprising what is the point of this ?
>
> Can I change its name to is_fraud ;-)
>
> --
> JSR
On Tuesday 20 January 2009, Jaswinder Singh Rajput wrote:
> CC: LKML
>
> On Tue, 2009-01-20 at 20:23 +0530, Jaswinder Singh Rajput wrote:
> > For if_frad.h I am getting two warnings with 'make headers_check':
> > usr/include/linux/if_frad.h:29: leaks CONFIG_DLCI to userspace where it is not valid
> > usr/include/linux/if_frad.h:129: leaks CONFIG_DLCI to userspace where it is not valid
> >
> > Almost all if_frad.h is covered with #if defined(CONFIG_DLCI) || defined(CONFIG_DLCI_MODULE)
> >
> > If I remove this portion so only this what we get in userspace:
> >
> > #ifndef _FRAD_H_
> > #define _FRAD_H_
> >
> > #include <linux/if.h>
> >
> > #endif
> >
> > So I am surprising what is the point of this ?
> >
> > Can I change its name to is_fraud ;-)
The CONFIG_DLCI check in there is clearly in error, the definition in there
is a user interface. The obvious solution is to drop the #if / #endif pair
without a replacement.
Arnd <><
Arnd Bergmann <[email protected]> writes:
> The CONFIG_DLCI check in there is clearly in error, the definition in there
> is a user interface. The obvious solution is to drop the #if / #endif pair
> without a replacement.
I think so.
OTOH I'd be really surprised if anyone on the planet still used this
code/driver. IIRC and AFAICS it is (sdla.c coupled with dlci.c) the
first driver for the old Sangoma ISA sync serial cards (S502 - S508,
i.e. those based on Z80CPU doing HDLC and other things in firmware).
There was also a second(?) driver for those cards (official from
Sangoma), but:
commit 8db60bcf3021921e2d10d158641792d640e52fe8
Author: Adrian Bunk <[email protected]>
Date: Tue Apr 11 17:28:33 2006 -0700
[WAN]: Remove broken and unmaintained Sangoma drivers.
The in-kernel Sangoma drivers are both not compiling and marked as BROKEN
since at least kernel 2.6.0.
Sangoma offers out-of-tree drivers, and David Mandelstam told me Sangoma
does no longer maintain the in-kernel drivers and prefers to provide them
as a separate installation package.
This patch therefore removes these drivers.
They claim support for "All S-Series Legacy Cards" with out-of-tree
drivers available from http://www.sangoma.com.
--
Krzysztof Halasa
On Tue, 2009-01-20 at 20:53 +0100, Krzysztof Halasa wrote:
> Arnd Bergmann <[email protected]> writes:
>
> > The CONFIG_DLCI check in there is clearly in error, the definition in there
> > is a user interface. The obvious solution is to drop the #if / #endif pair
> > without a replacement.
>
> I think so.
> OTOH I'd be really surprised if anyone on the planet still used this
> code/driver. IIRC and AFAICS it is (sdla.c coupled with dlci.c) the
> first driver for the old Sangoma ISA sync serial cards (S502 - S508,
> i.e. those based on Z80CPU doing HDLC and other things in firmware).
>
>
usr/include/linux/if_frad.h is covered with CONFIG_DLCI from many years
and no body is complaining about it so it means no body is using it.
So should we need to drop #if / #endif pair or the whole file from
userspace.
--
JSR
Jaswinder Singh Rajput <[email protected]> writes:
> usr/include/linux/if_frad.h is covered with CONFIG_DLCI from many years
> and no body is complaining about it so it means no body is using it.
>
> So should we need to drop #if / #endif pair or the whole file from
> userspace.
I think the file. "Empty file exported to userspace", long unused. We
can also have it back there, and it being not exported is an indicator
that it's not used. I guess the #ifdef __KERNEL__ can be removed, too.
--
Krzysztof Halasa
On Wed, Jan 21, 2009 at 6:36 AM, Krzysztof Halasa <[email protected]> wrote:
> Jaswinder Singh Rajput <[email protected]> writes:
>
>> usr/include/linux/if_frad.h is covered with CONFIG_DLCI from many years
>> and no body is complaining about it so it means no body is using it.
>>
>> So should we need to drop #if / #endif pair or the whole file from
>> userspace.
>
> I think the file. "Empty file exported to userspace", long unused. We
> can also have it back there, and it being not exported is an indicator
> that it's not used. I guess the #ifdef __KERNEL__ can be removed, too.
I will definately define #ifdef __KERNEL__ but I am also curious after
defining it there is no point of making empty
usr/include/linux/if_frad.h
--
JSR
On Wed, Jan 21, 2009 at 10:20:17AM +0530, Jaswinder Singh Rajput wrote:
> On Wed, Jan 21, 2009 at 6:36 AM, Krzysztof Halasa <[email protected]> wrote:
> > Jaswinder Singh Rajput <[email protected]> writes:
> >
> >> usr/include/linux/if_frad.h is covered with CONFIG_DLCI from many years
> >> and no body is complaining about it so it means no body is using it.
> >>
> >> So should we need to drop #if / #endif pair or the whole file from
> >> userspace.
> >
> > I think the file. "Empty file exported to userspace", long unused. We
> > can also have it back there, and it being not exported is an indicator
> > that it's not used. I guess the #ifdef __KERNEL__ can be removed, too.
>
> I will definately define #ifdef __KERNEL__ but I am also curious after
> defining it there is no point of making empty
> usr/include/linux/if_frad.h
Googling a bit did not turn up _any_ non-kernel hits that has relevance.
So based on the information given in this thread I strongly
suggest to drop the export of this header.
Sam
From: Sam Ravnborg <[email protected]>
Date: Wed, 21 Jan 2009 06:33:10 +0100
> On Wed, Jan 21, 2009 at 10:20:17AM +0530, Jaswinder Singh Rajput wrote:
> > On Wed, Jan 21, 2009 at 6:36 AM, Krzysztof Halasa <[email protected]> wrote:
> > > Jaswinder Singh Rajput <[email protected]> writes:
> > >
> > >> usr/include/linux/if_frad.h is covered with CONFIG_DLCI from many years
> > >> and no body is complaining about it so it means no body is using it.
> > >>
> > >> So should we need to drop #if / #endif pair or the whole file from
> > >> userspace.
> > >
> > > I think the file. "Empty file exported to userspace", long unused. We
> > > can also have it back there, and it being not exported is an indicator
> > > that it's not used. I guess the #ifdef __KERNEL__ can be removed, too.
> >
> > I will definately define #ifdef __KERNEL__ but I am also curious after
> > defining it there is no point of making empty
> > usr/include/linux/if_frad.h
>
> Googling a bit did not turn up _any_ non-kernel hits that has relevance.
> So based on the information given in this thread I strongly
> suggest to drop the export of this header.
Sure, but on the other hand this makes all of the userland APIs
essentially inaccessible and undefined.
I bet Sangoma's internal tools reference this stuff.
On Tue, 20 Jan 2009 22:15:04 -0800 (PST)
David Miller <[email protected]> wrote:
> From: Sam Ravnborg <[email protected]>
> Date: Wed, 21 Jan 2009 06:33:10 +0100
>
> > On Wed, Jan 21, 2009 at 10:20:17AM +0530, Jaswinder Singh Rajput wrote:
> > > On Wed, Jan 21, 2009 at 6:36 AM, Krzysztof Halasa <[email protected]> wrote:
> > > > Jaswinder Singh Rajput <[email protected]> writes:
> > > >
> > > >> usr/include/linux/if_frad.h is covered with CONFIG_DLCI from many years
> > > >> and no body is complaining about it so it means no body is using it.
> > > >>
> > > >> So should we need to drop #if / #endif pair or the whole file from
> > > >> userspace.
> > > >
> > > > I think the file. "Empty file exported to userspace", long unused. We
> > > > can also have it back there, and it being not exported is an indicator
> > > > that it's not used. I guess the #ifdef __KERNEL__ can be removed, too.
> > >
> > > I will definately define #ifdef __KERNEL__ but I am also curious after
> > > defining it there is no point of making empty
> > > usr/include/linux/if_frad.h
> >
> > Googling a bit did not turn up _any_ non-kernel hits that has relevance.
> > So based on the information given in this thread I strongly
> > suggest to drop the export of this header.
>
> Sure, but on the other hand this makes all of the userland APIs
> essentially inaccessible and undefined.
>
> I bet Sangoma's internal tools reference this stuff.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
There are no references if_frad.h in the version of Sangoma (out of tree) code
that we use in Vyatta.
From: Stephen Hemminger <[email protected]>
Date: Wed, 21 Jan 2009 17:38:38 +1100
> There are no references if_frad.h in the version of Sangoma (out of
> tree) code that we use in Vyatta.
frad-0.20:src/dlcicfg.c
It includes linux/if_frad.h and also performs the DLCI_SET_CONF
ioctl.
Enough evidence to show we need to keep this header in the
export set. Let's just kill the bogus CONFIG_* ifdef
tests in there.
On Wed, Jan 21, 2009 at 03:37:12PM -0800, David Miller wrote:
> From: Stephen Hemminger <[email protected]>
> Date: Wed, 21 Jan 2009 17:38:38 +1100
>
> > There are no references if_frad.h in the version of Sangoma (out of
> > tree) code that we use in Vyatta.
>
> frad-0.20:src/dlcicfg.c
>
> It includes linux/if_frad.h and also performs the DLCI_SET_CONF
> ioctl.
>
> Enough evidence to show we need to keep this header in the
> export set. Let's just kill the bogus CONFIG_* ifdef
> tests in there.
Thanks for investigating David.
Sam
On Wed, 2009-01-21 at 15:37 -0800, David Miller wrote:
> From: Stephen Hemminger <[email protected]>
> Date: Wed, 21 Jan 2009 17:38:38 +1100
>
> > There are no references if_frad.h in the version of Sangoma (out of
> > tree) code that we use in Vyatta.
>
> frad-0.20:src/dlcicfg.c
>
> It includes linux/if_frad.h and also performs the DLCI_SET_CONF
> ioctl.
>
> Enough evidence to show we need to keep this header in the
> export set. Let's just kill the bogus CONFIG_* ifdef
> tests in there.
After killing CONFIG_DLCI and CONFIG_DLCI_MODULE, it is also killing the
kernel:
CC net/socket.o
In file included from net/socket.c:75:
include/linux/if_frad.h:170: error: ‘CONFIG_DLCI_MAX’ undeclared here (not in a function)
make[1]: *** [net/socket.o] Error 1
make: *** [net] Error 2
because these things still depends on CONFIG_DLCI:
struct frad_local
{
struct net_device_stats stats;
/* devices which this FRAD is slaved to */
struct net_device *master[CONFIG_DLCI_MAX];
short dlci[CONFIG_DLCI_MAX];
Are you still sure we do not need CONFIG_DLCI.
--
JSR
Jaswinder Singh Rajput <[email protected]> writes:
> After killing CONFIG_DLCI and CONFIG_DLCI_MODULE, it is also killing the
> kernel:
>
> CC net/socket.o
> In file included from net/socket.c:75:
> include/linux/if_frad.h:170: error: ‘CONFIG_DLCI_MAX’ undeclared here (not in a function)
Perhaps this would do (untested).
Signed-off-by: Krzysztof Hałasa <[email protected]>
diff --git a/include/linux/if_frad.h b/include/linux/if_frad.h
index 5c34240..60e16a5 100644
--- a/include/linux/if_frad.h
+++ b/include/linux/if_frad.h
@@ -26,8 +26,6 @@
#include <linux/if.h>
-#if defined(CONFIG_DLCI) || defined(CONFIG_DLCI_MODULE)
-
/* Structures and constants associated with the DLCI device driver */
struct dlci_add
@@ -127,6 +125,8 @@ struct frad_conf
#ifdef __KERNEL__
+#if defined(CONFIG_DLCI) || defined(CONFIG_DLCI_MODULE)
+
/* these are the fields of an RFC 1490 header */
struct frhdr
{
@@ -190,12 +190,10 @@ struct frad_local
int buffer; /* current buffer for S508 firmware */
};
-#endif /* __KERNEL__ */
-
#endif /* CONFIG_DLCI || CONFIG_DLCI_MODULE */
-#ifdef __KERNEL__
extern void dlci_ioctl_set(int (*hook)(unsigned int, void __user *));
-#endif
+
+#endif /* __KERNEL__ */
#endif
From: Krzysztof Halasa <[email protected]>
Date: Sat, 24 Jan 2009 16:32:55 +0100
> Jaswinder Singh Rajput <[email protected]> writes:
>
> > After killing CONFIG_DLCI and CONFIG_DLCI_MODULE, it is also killing the
> > kernel:
> >
> > CC net/socket.o
> > In file included from net/socket.c:75:
> > include/linux/if_frad.h:170: error: ‘CONFIG_DLCI_MAX’ undeclared here (not in a function)
>
> Perhaps this would do (untested).
> Signed-off-by: Krzysztof Hałasa <[email protected]>
This looks good, applied, thanks everyone.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?