[PATCH] ax25.h unsigned long type for ax25 timers

[PATCH] ax25.h unsigned long type for ax25 timers

[Bernard Pidoux]

In linux/include/linux/ax25.h
ax25_info_struct timers types remain unsigned

struct ax25_info_struct {
	unsigned int	n2, n2count;
	unsigned int	t1, t1timer;
	unsigned int	t2, t2timer;
	unsigned int	t3, t3timer;
	unsigned int	idle, idletimer;
        .....
};

while in linux/include/net/ax25.h timers are unsigned long according
to kernel 2.6 timers.

typedef struct ax25_cb {
        .....
        struct timer_list       t1timer, t2timer, t3timer, idletimer;
        unsigned long           t1, t2, t3, idle, rtt;
        ....

Although ax25_info_struct is not much used it is refered at least
into libax25 and xfbb BBS application program.

It seems thus reasonable to make the change.

Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
---
 include/linux/ax25.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/ax25.h b/include/linux/ax25.h
index 56c11f0..75282bb 100644
--- a/include/linux/ax25.h
+++ b/include/linux/ax25.h
@@ -97,10 +97,10 @@ struct ax25_info_struct_deprecated {
 
 struct ax25_info_struct {
 	unsigned int	n2, n2count;
-	unsigned int	t1, t1timer;
-	unsigned int	t2, t2timer;
-	unsigned int	t3, t3timer;
-	unsigned int	idle, idletimer;
+	unsigned long	t1, t1timer;
+	unsigned long	t2, t2timer;
+	unsigned long	t3, t3timer;
+	unsigned long	idle, idletimer;
 	unsigned int	state;
 	unsigned int	rcv_q, snd_q;
 	unsigned int	vs, vr, va, vs_max;
-- 
1.5.5

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[David Miller]

From: Bernard Pidoux <bpidoux@free.fr>
Date: Tue, 17 Jun 2008 15:04:38 +0200

> In linux/include/linux/ax25.h
> ax25_info_struct timers types remain unsigned
> 
> struct ax25_info_struct {
> 	unsigned int	n2, n2count;
> 	unsigned int	t1, t1timer;
> 	unsigned int	t2, t2timer;
> 	unsigned int	t3, t3timer;
> 	unsigned int	idle, idletimer;
>         .....
> };
> 
> while in linux/include/net/ax25.h timers are unsigned long according
> to kernel 2.6 timers.
> 
> typedef struct ax25_cb {
>         .....
>         struct timer_list       t1timer, t2timer, t3timer, idletimer;
>         unsigned long           t1, t2, t3, idle, rtt;
>         ....
> 
> Although ax25_info_struct is not much used it is refered at least
> into libax25 and xfbb BBS application program.
> 
> It seems thus reasonable to make the change.
> 
> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>

Unfortunately this datastructure is exported to userspace,
and therefore we cannot change the structure layout without
breaking userspace.

We cannot, as a result, make this change.

The only way to fix this is to make a new fixed structure, and add new
ax25 calls that accept this new structure.

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[Bernard Pidoux]

David,

Unfortunately I don't quite understand exactly why a structure in user 
space cannot be changed.
But this means that I have to make some effort to learn a bit more about 
the kernel structure and userspace.
However, thank you for your explanation about how to fix the discrepancy 
between both timer structures.
I will try to dig out more information on this subject.

Bernard Pidoux, f6bvp
 

David Miller a écrit :
> From: Bernard Pidoux <bpidoux@free.fr>
> Date: Tue, 17 Jun 2008 15:04:38 +0200
>
>   
>> In linux/include/linux/ax25.h
>> ax25_info_struct timers types remain unsigned
>>
>> struct ax25_info_struct {
>> 	unsigned int	n2, n2count;
>> 	unsigned int	t1, t1timer;
>> 	unsigned int	t2, t2timer;
>> 	unsigned int	t3, t3timer;
>> 	unsigned int	idle, idletimer;
>>         .....
>> };
>>
>> while in linux/include/net/ax25.h timers are unsigned long according
>> to kernel 2.6 timers.
>>
>> typedef struct ax25_cb {
>>         .....
>>         struct timer_list       t1timer, t2timer, t3timer, idletimer;
>>         unsigned long           t1, t2, t3, idle, rtt;
>>         ....
>>
>> Although ax25_info_struct is not much used it is refered at least
>> into libax25 and xfbb BBS application program.
>>
>> It seems thus reasonable to make the change.
>>
>> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
>>     
>
> Unfortunately this datastructure is exported to userspace,
> and therefore we cannot change the structure layout without
> breaking userspace.
>
> We cannot, as a result, make this change.
>
> The only way to fix this is to make a new fixed structure, and add new
> ax25 calls that accept this new structure.
>
>
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-hams" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[Al]

David
My impression was that the proposed change did NOT change binary 
interfaces to the kernel, but correctly labeled fields for the first 
time specifically as "long" instead of the fuzzy definition as "int".
While it's true that a non-386 system where "int" is not a "long" would 
be adversely affected, there appear not to be any of those (rare) 
systems running this software, so the point seems entirely moot.
The proposed change improves strict type checking and makes a cleaner 
definition.
I say let it in.
Al

David Miller wrote:
> From: Bernard Pidoux <bpidoux@free.fr>
> Date: Tue, 17 Jun 2008 15:04:38 +0200
>
>   
>> In linux/include/linux/ax25.h
>> ax25_info_struct timers types remain unsigned
>>
>> struct ax25_info_struct {
>> 	unsigned int	n2, n2count;
>> 	unsigned int	t1, t1timer;
>> 	unsigned int	t2, t2timer;
>> 	unsigned int	t3, t3timer;
>> 	unsigned int	idle, idletimer;
>>         .....
>> };
>>
>> while in linux/include/net/ax25.h timers are unsigned long according
>> to kernel 2.6 timers.
>>
>> typedef struct ax25_cb {
>>         .....
>>         struct timer_list       t1timer, t2timer, t3timer, idletimer;
>>         unsigned long           t1, t2, t3, idle, rtt;
>>         ....
>>
>> Although ax25_info_struct is not much used it is refered at least
>> into libax25 and xfbb BBS application program.
>>
>> It seems thus reasonable to make the change.
>>
>> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
>>     
>
> Unfortunately this datastructure is exported to userspace,
> and therefore we cannot change the structure layout without
> breaking userspace.
>
> We cannot, as a result, make this change.
>
> The only way to fix this is to make a new fixed structure, and add new
> ax25 calls that accept this new structure.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hams" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[Al]

Actually on closer inspection, the items are reordered, which would 
break existing compiled programs, so I'll have to reverse myself on this.
Sorry for the distraction.
Al

Al wrote:
> David
> My impression was that the proposed change did NOT change binary 
> interfaces to the kernel, but correctly labeled fields for the first 
> time specifically as "long" instead of the fuzzy definition as "int".
> While it's true that a non-386 system where "int" is not a "long" 
> would be adversely affected, there appear not to be any of those 
> (rare) systems running this software, so the point seems entirely moot.
> The proposed change improves strict type checking and makes a cleaner 
> definition.
> I say let it in.
> Al
>
> David Miller wrote:
>> From: Bernard Pidoux <bpidoux@free.fr>
>> Date: Tue, 17 Jun 2008 15:04:38 +0200
>>
>>  
>>> In linux/include/linux/ax25.h
>>> ax25_info_struct timers types remain unsigned
>>>
>>> struct ax25_info_struct {
>>>     unsigned int    n2, n2count;
>>>     unsigned int    t1, t1timer;
>>>     unsigned int    t2, t2timer;
>>>     unsigned int    t3, t3timer;
>>>     unsigned int    idle, idletimer;
>>>         .....
>>> };
>>>
>>> while in linux/include/net/ax25.h timers are unsigned long according
>>> to kernel 2.6 timers.
>>>
>>> typedef struct ax25_cb {
>>>         .....
>>>         struct timer_list       t1timer, t2timer, t3timer, idletimer;
>>>         unsigned long           t1, t2, t3, idle, rtt;
>>>         ....
>>>
>>> Although ax25_info_struct is not much used it is refered at least
>>> into libax25 and xfbb BBS application program.
>>>
>>> It seems thus reasonable to make the change.
>>>
>>> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
>>>     
>>
>> Unfortunately this datastructure is exported to userspace,
>> and therefore we cannot change the structure layout without
>> breaking userspace.
>>
>> We cannot, as a result, make this change.
>>
>> The only way to fix this is to make a new fixed structure, and add new
>> ax25 calls that accept this new structure.
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-hams" 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-hams" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[David Miller]

From: Bernard Pidoux <bpidoux@free.fr>
Date: Thu, 19 Jun 2008 18:28:46 +0200

> Unfortunately I don't quite understand exactly why a structure in user 
> space cannot be changed.

Because there are userland binaries already compiled out there using
the current structure layout.  If we change the kernel, those binaries
stop using a datastructure that matches what the kernel is using.
Such binaries will break.

In generic, because of this, you can never change the layout of a
data structure exported to userland as a kernel interface.

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[Bernard Pidoux]

David,

Your explanation is perfectly clear and I understand the problem now.
Thank you very much for taking the time to explain me that.

Bernard Pidoux




David Miller wrote:
> From: Bernard Pidoux <bpidoux@free.fr>
> Date: Thu, 19 Jun 2008 18:28:46 +0200
> 
>> Unfortunately I don't quite understand exactly why a structure in user 
>> space cannot be changed.
> 
> Because there are userland binaries already compiled out there using
> the current structure layout.  If we change the kernel, those binaries
> stop using a datastructure that matches what the kernel is using.
> Such binaries will break.
> 
> In generic, because of this, you can never change the layout of a
> data structure exported to userland as a kernel interface.
> 
> 

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[Bernard Pidoux F6BVP]

There have been in the past already some modifications of 
ax25_info_struct and the old one was renamed deprecated.

/* this will go away. Please do not export to user land */
struct ax25_info_struct_deprecated {
         unsigned int    n2, n2count;
         unsigned int    t1, t1timer;
         unsigned int    t2, t2timer;
         unsigned int    t3, t3timer;
         unsigned int    idle, idletimer;
         unsigned int    state;
         unsigned int    rcv_q, snd_q;
};

Three new items were included :

...
         unsigned int    vs, vr, va, vs_max;
         unsigned int    paclen;
         unsigned int    window;
};

At the same time  case SIOCAX25GETINFOOLD:
was added in ax25_ioctl()
and would trigger a warning message.


  /* old structure? */
                 if (cmd == SIOCAX25GETINFOOLD) {
                         static int warned = 0;
                         if (!warned) {
                                 printk(KERN_INFO "%s uses old 
SIOCAX25GETINFO\n",
                                         current->comm);
                                 warned=1;
                         }

                         if (copy_to_user(argp, &ax25_info, 
sizeof(struct ax25_info_struct_deprecated))) {
                                 res = -EFAULT;
                                 break;
                         }
                 } else {
                         if (copy_to_user(argp, &ax25_info, 
sizeof(struct ax25_info_struct))) {
                                 res = -EINVAL;
                                 break;
                         }
                 }
                 res = 0;
                 break;

Here is my question :

Can't we remove the probably quite old ax25_info_struct_deprecated 
structure, and rename the present ax25_info_struct, 
ax25_info_struct_deprecated, then create the new ax25_info_struct with 
resized timers unsigned long ?

The second solution is to declare a new structure

* new ax25 info struct */
struct ax25_info_long_timers_struct {
         unsigned int    n2, n2count;
         unsigned long   t1, t1timer;
         unsigned long   t2, t2timer;
         unsigned long   t3, t3timer;
         unsigned long   idle, idletimer;
         unsigned int    state;
         unsigned int    rcv_q, snd_q;
         unsigned int    vs, vr, va, vs_max;
         unsigned int    paclen;
         unsigned int    window;
};

adding another case SIOCAX25GETNEWINFO
that would be defined
#define SIOCAX25GETNEWINFO         (SIOCPROTOPRIVATE+14)

and trigger warning message by calls to SIOCAX25GETINFOOLD and
SIOCAX25GETINFO.

It that the right way to do it ?


Bernard, f6bvp



Bernard Pidoux wrote:
> David,
> 
> Your explanation is perfectly clear and I understand the problem now.
> Thank you very much for taking the time to explain me that.
> 
> Bernard Pidoux
> 
> 
> 
> 
> David Miller wrote:
>> From: Bernard Pidoux <bpidoux@free.fr>
>> Date: Thu, 19 Jun 2008 18:28:46 +0200
>>
>>> Unfortunately I don't quite understand exactly why a structure in 
>>> user space cannot be changed.
>>
>> Because there are userland binaries already compiled out there using
>> the current structure layout.  If we change the kernel, those binaries
>> stop using a datastructure that matches what the kernel is using.
>> Such binaries will break.
>>
>> In generic, because of this, you can never change the layout of a
>> data structure exported to userland as a kernel interface.
>>
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-hams" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[David Miller]

From: Bernard Pidoux F6BVP <f6bvp@free.fr>
Date: Fri, 20 Jun 2008 15:17:39 +0200

> The second solution is to declare a new structure
> 
> * new ax25 info struct */
> struct ax25_info_long_timers_struct {
 ...
> 
> adding another case SIOCAX25GETNEWINFO
> that would be defined
> #define SIOCAX25GETNEWINFO         (SIOCPROTOPRIVATE+14)
> 
> and trigger warning message by calls to SIOCAX25GETINFOOLD and
> SIOCAX25GETINFO.
> 
> It that the right way to do it ?

Yes.