Re: a question regarding sys/shm.h
On Thu, 15 Feb 2007, Pascal Hofstee wrote: On Thu, 2007-02-15 at 13:41 +, Robert Watson wrote: Unfortunately, things are a bit more tricky. The problem is not so much the API, where converting size_t/int is a relative non-event, rather, the ABI. By changing the size of a field in a data structure, you may change the layout of the structure, and hence the offset of other fields. This offset information is compiled into binaries that access the structure -- hence being part of the ABI. On i386, the change from int to size_t doesn't modify the ABI, as both int and size_t are 32-bit. However, on 64-bit platforms, int is 32-bit and size_t is 64-bit: sledge:/tmp uname -a FreeBSD sledge.freebsd.org 7.0-CURRENT FreeBSD 7.0-CURRENT #898: Wed Feb 14 14:20:16 UTC 2007 [EMAIL PROTECTED]:/h/src/sys/amd64/compile/SLEDGE amd64 sledge:/tmp ./size_t sizeof int: 4 sizeof size_t: 8 In practice, this means that all of the later fields in the data structure will be offset by 4 bytes. This will affect any application that accesses later fields in the structure but isn't recompiled. This is why DES and I have been discussing this change as requiring kernel compatibility code, which would provide new system calls working with the new layout, and retain old system calls working with the old layout. So we'd need to provide a new shmctl() with the new structure, and an oshmctl() with the old layout. While doing that, it makes sense to do all the other ABI-related things that we'd like to get out of the way, such as fixing the types in shm_perm. I understand ... i'll leave this up to you guys .. you have obviously a lot more hands on experience in these kinds of matters :) Well -- don't let this discourage you from working on it, I'm just pointing out that there are some more details to work on before it will be done :-). I'm happy to advise further as the work moves along, but unfortunately don't have time to do it myself at this point. It's something I would very much like to see happen, though! Robert N M Watson Computer Laboratory University of Cambridge ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: a question regarding sys/shm.h
On 1/31/07, Robert Watson [EMAIL PROTECTED] wrote:
If we do decide to go ahead with the ABI change, there are a number of other
things that should be done simultaneously, such as changing the uid and gid
fields to uid_t and gid_t. I would very much like to see the ABI change
happen, and the first step (breaking out kernel from user structures) has been
done already as part of the MAC work. The next step is to add routines that
translate internal/external formats, which isn't hard, but requires a moderate
pile of code to do (as well as great care :-).
Well .. i finally found some spare time to have a closer look at the
shm_segsz issue ... and noticed there were actually a very limited
number of direct uses of the shm_segsz struct member (26 lines in the
entire /usr/src tree)
I have attached a patchset that should change shm_segsz to size_t.
There were however 2 to 3 locations all regarding compat code (ibcs2,
svr4 and COMPAT_43) where i opted to stay on the clear side and not
touch anything, the rest was fairly straightforward as should be
obvious from the diff. I checked to make sure no function prototypes
changed anywhere.
Please have a look at the attached patch (available at
http://callisto.offis.uni-oldenburg.de/shm_segsz-int2size_t.diff in
case the attachment gets stripped off by the mailinglist software) and
provide any feedback where appropriate.
--
Pascal Hofstee
diff -rwuBb src.ref/lib/libc/sys/shmctl.2 src/lib/libc/sys/shmctl.2
--- src.ref/lib/libc/sys/shmctl.2 Thu Feb 15 09:55:01 2007
+++ src/lib/libc/sys/shmctl.2 Thu Feb 15 09:56:49 2007
@@ -100,7 +100,7 @@
.Bd -literal
struct shmid_ds {
struct ipc_perm shm_perm; /* operation permission structure */
-int shm_segsz; /* size of segment in bytes */
+size_t shm_segsz; /* size of segment in bytes */
pid_t shm_lpid; /* process ID of last shared memory op */
pid_t shm_cpid; /* process ID of creator */
short shm_nattch; /* number of current attaches */
diff -rwuBb src.ref/sys/compat/freebsd32/freebsd32_misc.c src/sys/compat/freebsd32/freebsd32_misc.c
--- src.ref/sys/compat/freebsd32/freebsd32_misc.c Thu Feb 15 09:49:53 2007
+++ src/sys/compat/freebsd32/freebsd32_misc.c Thu Feb 15 09:59:05 2007
@@ -1445,7 +1445,7 @@
};
struct shmid_ds32 {
struct ipc_perm32 shm_perm;
- int32_t shm_segsz;
+ size_t shm_segsz;
int32_t shm_lpid;
int32_t shm_cpid;
int16_t shm_nattch;
diff -rwuBb src.ref/sys/compat/linux/linux_ipc.c src/sys/compat/linux/linux_ipc.c
--- src.ref/sys/compat/linux/linux_ipc.c Thu Feb 15 09:49:53 2007
+++ src/sys/compat/linux/linux_ipc.c Thu Feb 15 10:03:43 2007
@@ -187,7 +187,7 @@
struct l_shmid_ds {
struct l_ipc_perm shm_perm;
- l_int shm_segsz;
+ l_size_t shm_segsz;
l_time_t shm_atime;
l_time_t shm_dtime;
l_time_t shm_ctime;
diff -rwuBb src.ref/sys/sys/shm.h src/sys/sys/shm.h
--- src.ref/sys/sys/shm.h Thu Feb 15 09:52:13 2007
+++ src/sys/sys/shm.h Thu Feb 15 10:14:56 2007
@@ -77,7 +77,7 @@
struct shmid_ds {
struct ipc_perm shm_perm; /* operation permission structure */
- int shm_segsz; /* size of segment in bytes */
+ size_t shm_segsz; /* size of segment in bytes */
pid_t shm_lpid; /* process ID of last shared memory op */
pid_t shm_cpid; /* process ID of creator */
short shm_nattch; /* number of current attaches */
diff -rwuBb src.ref/tools/regression/sysvshm/shmtest.c src/tools/regression/sysvshm/shmtest.c
--- src.ref/tools/regression/sysvshm/shmtest.c Thu Feb 15 09:49:36 2007
+++ src/tools/regression/sysvshm/shmtest.c Thu Feb 15 10:22:24 2007
@@ -247,8 +247,8 @@
sp-shm_perm.cuid, sp-shm_perm.cgid,
sp-shm_perm.mode 0777);
- printf(segsz %lu, lpid %d, cpid %d, nattch %u\n,
- (u_long)sp-shm_segsz, sp-shm_lpid, sp-shm_cpid,
+ printf(segsz %zu, lpid %d, cpid %d, nattch %u\n,
+ sp-shm_segsz, sp-shm_lpid, sp-shm_cpid,
sp-shm_nattch);
printf(atime: %s, ctime(sp-shm_atime));
diff -rwuBb src.ref/usr.bin/ipcs/ipcs.c src/usr.bin/ipcs/ipcs.c
--- src.ref/usr.bin/ipcs/ipcs.c Thu Feb 15 09:49:15 2007
+++ src/usr.bin/ipcs/ipcs.c Thu Feb 15 10:20:27 2007
@@ -439,7 +439,7 @@
kshmptr-u.shm_nattch);
if (option BIGGEST)
- printf( %12d,
+ printf( %12zu,
kshmptr-u.shm_segsz);
if (option PID)
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: a question regarding sys/shm.h
On Thu, 15 Feb 2007, Pascal Hofstee wrote: On 1/31/07, Robert Watson [EMAIL PROTECTED] wrote: If we do decide to go ahead with the ABI change, there are a number of other things that should be done simultaneously, such as changing the uid and gid fields to uid_t and gid_t. I would very much like to see the ABI change happen, and the first step (breaking out kernel from user structures) has been done already as part of the MAC work. The next step is to add routines that translate internal/external formats, which isn't hard, but requires a moderate pile of code to do (as well as great care :-). Well .. i finally found some spare time to have a closer look at the shm_segsz issue ... and noticed there were actually a very limited number of direct uses of the shm_segsz struct member (26 lines in the entire /usr/src tree) I have attached a patchset that should change shm_segsz to size_t. There were however 2 to 3 locations all regarding compat code (ibcs2, svr4 and COMPAT_43) where i opted to stay on the clear side and not touch anything, the rest was fairly straightforward as should be obvious from the diff. I checked to make sure no function prototypes changed anywhere. Please have a look at the attached patch (available at http://callisto.offis.uni-oldenburg.de/shm_segsz-int2size_t.diff in case the attachment gets stripped off by the mailinglist software) and provide any feedback where appropriate. Unfortunately, things are a bit more tricky. The problem is not so much the API, where converting size_t/int is a relative non-event, rather, the ABI. By changing the size of a field in a data structure, you may change the layout of the structure, and hence the offset of other fields. This offset information is compiled into binaries that access the structure -- hence being part of the ABI. On i386, the change from int to size_t doesn't modify the ABI, as both int and size_t are 32-bit. However, on 64-bit platforms, int is 32-bit and size_t is 64-bit: sledge:/tmp uname -a FreeBSD sledge.freebsd.org 7.0-CURRENT FreeBSD 7.0-CURRENT #898: Wed Feb 14 14:20:16 UTC 2007 [EMAIL PROTECTED]:/h/src/sys/amd64/compile/SLEDGE amd64 sledge:/tmp ./size_t sizeof int: 4 sizeof size_t: 8 In practice, this means that all of the later fields in the data structure will be offset by 4 bytes. This will affect any application that accesses later fields in the structure but isn't recompiled. This is why DES and I have been discussing this change as requiring kernel compatibility code, which would provide new system calls working with the new layout, and retain old system calls working with the old layout. So we'd need to provide a new shmctl() with the new structure, and an oshmctl() with the old layout. While doing that, it makes sense to do all the other ABI-related things that we'd like to get out of the way, such as fixing the types in shm_perm. Robert N M Watson Computer Laboratory University of Cambridge ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: a question regarding sys/shm.h
On Thu, 2007-02-15 at 13:41 +, Robert Watson wrote: Unfortunately, things are a bit more tricky. The problem is not so much the API, where converting size_t/int is a relative non-event, rather, the ABI. By changing the size of a field in a data structure, you may change the layout of the structure, and hence the offset of other fields. This offset information is compiled into binaries that access the structure -- hence being part of the ABI. On i386, the change from int to size_t doesn't modify the ABI, as both int and size_t are 32-bit. However, on 64-bit platforms, int is 32-bit and size_t is 64-bit: sledge:/tmp uname -a FreeBSD sledge.freebsd.org 7.0-CURRENT FreeBSD 7.0-CURRENT #898: Wed Feb 14 14:20:16 UTC 2007 [EMAIL PROTECTED]:/h/src/sys/amd64/compile/SLEDGE amd64 sledge:/tmp ./size_t sizeof int: 4 sizeof size_t: 8 In practice, this means that all of the later fields in the data structure will be offset by 4 bytes. This will affect any application that accesses later fields in the structure but isn't recompiled. This is why DES and I have been discussing this change as requiring kernel compatibility code, which would provide new system calls working with the new layout, and retain old system calls working with the old layout. So we'd need to provide a new shmctl() with the new structure, and an oshmctl() with the old layout. While doing that, it makes sense to do all the other ABI-related things that we'd like to get out of the way, such as fixing the types in shm_perm. I understand ... i'll leave this up to you guys .. you have obviously a lot more hands on experience in these kinds of matters :) -- Pascal Hofstee ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: a question regarding sys/shm.h
In message: [EMAIL PROTECTED]
Fabian Keil [EMAIL PROTECTED] writes:
: Robert Watson [EMAIL PROTECTED] wrote:
:
: On Wed, 31 Jan 2007, Dag-Erling Smørgrav wrote:
:
: Pascal Hofstee [EMAIL PROTECTED] writes:
: Any additional sugestions/objections are always greatly appreciated.
:
: On 32-bit platforms (i386, powerpc), int is a 32-bit signed integer while
: size_t is a 32-bit unsigned integer.
:
: On 64-bit platforms (amd64, sparc64 etc), int is a 32-bit signed integer
: while size_t is a 64-bit unsigned integer.
:
: In both cases, changing this structure member from int to size_t will
break
: the ABI.
:
: This doesn't mean you shouldn't do it, just that it should be done with
: care.
:
: If we do decide to go ahead with the ABI change, there are a number of
other
: things that should be done simultaneously, such as changing the uid and gid
: fields to uid_t and gid_t.
:
: struct tm's members could be changed as well.
struct tm doesn't have a member that's tv_sec.
Maybe you mean struct timeval and struct timespec?
: Quoting a response I got from Juliusz Chroboczek
: (the author of Polipo) after reporting a compiler
: warning:
:
: |By the way, IEEE 1003.1-2003 says that tv_sec should be a time_t,
: |hence by making it a long, FreeBSD and NetBSD are violating the POSIX
: |standard. Could you please file a bug report with them?
:
: I didn't find any claims about FreeBSD being IEEE 1003.1-2003
: compliant and therefore didn't consider it a bug, but given that
: the topic is *_t changes and time_t hasn't come up yet,
: I'd like to mention it anyway.
I think this is already fixed:
/*
* Structure returned by gettimeofday(2) system call, and used in other calls.
*/
struct timeval {
time_t tv_sec; /* seconds */
suseconds_t tv_usec;/* and microseconds */
};
as is timespec:
struct timespec {
time_t tv_sec; /* seconds */
longtv_nsec;/* and nanoseconds */
};
Warner
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: a question regarding sys/shm.h
M. Warner Losh [EMAIL PROTECTED] wrote: In message: [EMAIL PROTECTED] Fabian Keil [EMAIL PROTECTED] writes: : Robert Watson [EMAIL PROTECTED] wrote: : If we do decide to go ahead with the ABI change, there are a number of other : things that should be done simultaneously, such as changing the uid and gid : fields to uid_t and gid_t. : : struct tm's members could be changed as well. struct tm doesn't have a member that's tv_sec. Maybe you mean struct timeval and struct timespec? I did indeed. Sorry for the noise. Fabian signature.asc Description: PGP signature
Re: a question regarding sys/shm.h
On Wed, 2007-Jan-31 08:30:27 +0100, Pascal Hofstee wrote: In a recent attempt in trying to clean up some compiler warnings in a GNUstep related project i came upon a case where the FreeBSD datatypes seemed to disagree with the Linux ones. Though this in itself is not unusual i do wonder if in this case the Linux definition isn't the more proper one. The definition in question is inside sys/shm.h and involves struct shmid_ds.shm_segsz which seems to be defined as int whereas Linux defines this as size_t. Whilst I agree that the Linux defn is the more sensible one, System V IPC and common sense are not commonly found together. Tradionally the definition was int. It appears that the definition changed from int to size_t in issue 5 of the Open Group base definition but FreeBSD has not caught up with this. I'm not sure what plans there are to change this. You could try putting together a patch to address this and submitting it as a PR (this means addressing all references to shm_segsz in the base system, not just sys/shm.h). -- Peter Jeremy pgp8hjEZLqbWd.pgp Description: PGP signature
Re: a question regarding sys/shm.h
Peter Jeremy wrote: Whilst I agree that the Linux defn is the more sensible one, System V IPC and common sense are not commonly found together. Tradionally the definition was int. It appears that the definition changed from int to size_t in issue 5 of the Open Group base definition but FreeBSD has not caught up with this. I'm not sure what plans there are to change this. You could try putting together a patch to address this and submitting it as a PR (this means addressing all references to shm_segsz in the base system, not just sys/shm.h). First of all, thanks for the quick response. At the very least this sheds some light onto the history and how this situation likely came to pass. I'll see if i can put together a patch during the next week or so that would bring our definitions regarding System V IPC in synch, i'll also try to see if i can find additional occurances of similar discrepancies so these could be fixed in a single run. Any additional sugestions/objections are always greatly appreciated. -- Pascal Hofstee ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: a question regarding sys/shm.h
Pascal Hofstee [EMAIL PROTECTED] writes: Any additional sugestions/objections are always greatly appreciated. On 32-bit platforms (i386, powerpc), int is a 32-bit signed integer while size_t is a 32-bit unsigned integer. On 64-bit platforms (amd64, sparc64 etc), int is a 32-bit signed integer while size_t is a 64-bit unsigned integer. In both cases, changing this structure member from int to size_t will break the ABI. This doesn't mean you shouldn't do it, just that it should be done with care. DES -- Dag-Erling Smørgrav - [EMAIL PROTECTED] ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: a question regarding sys/shm.h
On Wed, 31 Jan 2007, Dag-Erling Smørgrav wrote: Pascal Hofstee [EMAIL PROTECTED] writes: Any additional sugestions/objections are always greatly appreciated. On 32-bit platforms (i386, powerpc), int is a 32-bit signed integer while size_t is a 32-bit unsigned integer. On 64-bit platforms (amd64, sparc64 etc), int is a 32-bit signed integer while size_t is a 64-bit unsigned integer. In both cases, changing this structure member from int to size_t will break the ABI. This doesn't mean you shouldn't do it, just that it should be done with care. If we do decide to go ahead with the ABI change, there are a number of other things that should be done simultaneously, such as changing the uid and gid fields to uid_t and gid_t. I would very much like to see the ABI change happen, and the first step (breaking out kernel from user structures) has been done already as part of the MAC work. The next step is to add routines that translate internal/external formats, which isn't hard, but requires a moderate pile of code to do (as well as great care :-). Robert N M Watson Computer Laboratory University of Cambridge___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: a question regarding sys/shm.h
Robert Watson [EMAIL PROTECTED] wrote: On Wed, 31 Jan 2007, Dag-Erling Smørgrav wrote: Pascal Hofstee [EMAIL PROTECTED] writes: Any additional sugestions/objections are always greatly appreciated. On 32-bit platforms (i386, powerpc), int is a 32-bit signed integer while size_t is a 32-bit unsigned integer. On 64-bit platforms (amd64, sparc64 etc), int is a 32-bit signed integer while size_t is a 64-bit unsigned integer. In both cases, changing this structure member from int to size_t will break the ABI. This doesn't mean you shouldn't do it, just that it should be done with care. If we do decide to go ahead with the ABI change, there are a number of other things that should be done simultaneously, such as changing the uid and gid fields to uid_t and gid_t. struct tm's members could be changed as well. Quoting a response I got from Juliusz Chroboczek (the author of Polipo) after reporting a compiler warning: |By the way, IEEE 1003.1-2003 says that tv_sec should be a time_t, |hence by making it a long, FreeBSD and NetBSD are violating the POSIX |standard. Could you please file a bug report with them? I didn't find any claims about FreeBSD being IEEE 1003.1-2003 compliant and therefore didn't consider it a bug, but given that the topic is *_t changes and time_t hasn't come up yet, I'd like to mention it anyway. Fabian signature.asc Description: PGP signature
Re: a question regarding sys/shm.h
On Wed, 2007-Jan-31 10:52:02 +, Robert Watson wrote: If we do decide to go ahead with the ABI change, there are a number of other things that should be done simultaneously, such as changing the uid and gid fields to uid_t and gid_t. And mode to mode_t. The uid and gid fields in struct shmid_ds have already been converted, though the ones in struct ipc_perm are obsolete. At a quick glance, everything else is up to date. http://www.opengroup.org/onlinepubs/009695399/ is a useful reference in this area. -- Peter Jeremy pgpQdwA5x7yOc.pgp Description: PGP signature
a question regarding sys/shm.h
Hi, In a recent attempt in trying to clean up some compiler warnings in a GNUstep related project i came upon a case where the FreeBSD datatypes seemed to disagree with the Linux ones. Though this in itself is not unusual i do wonder if in this case the Linux definition isn't the more proper one. The definition in question is inside sys/shm.h and involves struct shmid_ds.shm_segsz which seems to be defined as int whereas Linux defines this as size_t. I understand these definitions are usually platform dependent but am wondering if Linux's size_t wouldn't be a more proper type for this field .. and if it would make sense to perhaps synchronize our datatypes used here with those used by Linux? With kind regards, -- Pascal Hofstee ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
