-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
the recent commit 78b81aa3cb90 on the puredata/master branch breaks ABI compatibility with externals compiled for Pd-0.43.
the problem is that the "sys_close()" symbol is removed for non-w32 platforms. therefore all the externals on non-w32 that (already) use sys_close() (at least i have written a couple of them) will fail to load with a new version of Pd, unless they are recompiled.
this makes packaging externals for e.g. Debian a nightmare, as it basically should trigger a .so-name change, but since we are linking against the application instead of an ordinary library, all the tools that would detect such an incompatibility will fail.
so please revert the "#define sys_close close" stanzas.
instead i would ask you to provide sys_open() (and friends) implementations in s_path, even for platforms where they are mere wrappers around the system functions.
it also makes the header-file much easier to read (i don't think anything in a public header-file but function decorations should be ifdef'ed)
fgamsdr IOhannes
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 2012-12-17 10:55, IOhannes m zmoelnig wrote:
this makes packaging externals for e.g. Debian a nightmare, as it basically should trigger a .so-name change, but since we are linking against the application instead of an ordinary library, all the tools that would detect such an incompatibility will fail.
this is not only theoretical, but already happened: the Gem" binary as currently packaged in Debian cannot be loaded anymore with the git/master version of puredata.
so please revert the "#define sys_close close" stanzas.
instead i would ask you to provide sys_open() (and friends) implementations in s_path, even for platforms where they are mere wrappers around the system functions.
i created a patch on sourceforge that implements sys_f?(open,close) on all platforms and thus re-establishes binary compatibility.
the new functions are simply wrappers around the system open/close functions. the open wrappers also use sys_bashfilename (like the w32 version), which should be quite a noop on non-w32 platforms for now.
see: https://sourceforge.net/tracker/?group_id=55736&atid=478072
fgmasdr IOhannes
On Dec 17, 2012, at 6:05 AM, IOhannes m zmoelnig wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 2012-12-17 10:55, IOhannes m zmoelnig wrote:
this makes packaging externals for e.g. Debian a nightmare, as it basically should trigger a .so-name change, but since we are linking against the application instead of an ordinary library, all the tools that would detect such an incompatibility will fail.
this is not only theoretical, but already happened: the Gem" binary as currently packaged in Debian cannot be loaded anymore with the git/master version of puredata.
so please revert the "#define sys_close close" stanzas.
instead i would ask you to provide sys_open() (and friends) implementations in s_path, even for platforms where they are mere wrappers around the system functions.
i created a patch on sourceforge that implements sys_f?(open,close) on all platforms and thus re-establishes binary compatibility.
the new functions are simply wrappers around the system open/close functions. the open wrappers also use sys_bashfilename (like the w32 version), which should be quite a noop on non-w32 platforms for now.
see: https://sourceforge.net/tracker/?group_id=55736&atid=478072
I think this approach works. The patch you provided seems totally untested, as in not even compiled on GNU/Linux or Mac OS X. It includes the _close() function in sys_close() which only works on Win32 and it gives this warning when building on Mac OS X:
s_path.c: In function ‘sys_open’: s_path.c:450: warning: ‘mode_t’ is promoted to ‘int’ when passed through ‘...’ s_path.c:450: warning: (so you should pass ‘int’ not ‘mode_t’ to ‘va_arg’) s_path.c:450: note: if this code is reached, the program will abort
(I attached the patch for reference, it doesn't seem to be up on the patch tracker any more.)
.hc
On 12/18/2012 04:40, Hans-Christoph Steiner wrote:
I think this approach works.
thanks
The patch you provided seems totally untested, as in not even compiled on GNU/Linux or Mac OS X. It includes the _close() function in sys_close() which only works on Win32 and it gives this warning when building on Mac OS X:
thanks for the compliments :-)
i can assure you that the code is tested as in "compiled without warning on gcc-4.7.2 on a debian/gnu linux (wheezy/sid) system" and has been field-tested and shown to be able to load externals that the old code has not been able to load.
however, you are of course right that the use of "_close()" is indeed an oversight, which only did not trigger a problem so far, as sys_close() is nowhere used yet.
s_path.c: In function ‘sys_open’: s_path.c:450: warning: ‘mode_t’ is promoted to ‘int’ when passed through ‘...’ s_path.c:450: warning: (so you should pass ‘int’ not ‘mode_t’ to ‘va_arg’) s_path.c:450: note: if this code is reached, the program will abort
the patch includes some comments pointing to an online discussion of the problem. to summarize: using mode_t in va_list will always trigger some problems. either we accept the warning (the code will never be reached, since a runtime-test will use a va_arg(..., int) instead) or we move the test to configure (autoconf).
since i am the only one who seems to like autoconf, i decided for the less invasive solution.
(I attached the patch for reference, it doesn't seem to be up on the patch tracker any more.)
it seems that the patch has moved to the "bug" tracker. i moved it back to "patches" [1].
fgmasdr IOhannes
[1] https://sourceforge.net/tracker/?func=detail&aid=3596865&group_id=55...
On Dec 18, 2012, at 4:56 AM, IOhannes m zmölnig wrote:
On 12/18/2012 04:40, Hans-Christoph Steiner wrote:
I think this approach works.
thanks
The patch you provided seems totally untested, as in not even compiled on GNU/Linux or Mac OS X. It includes the _close() function in sys_close() which only works on Win32 and it gives this warning when building on Mac OS X:
thanks for the compliments :-)
i can assure you that the code is tested as in "compiled without warning on gcc-4.7.2 on a debian/gnu linux (wheezy/sid) system" and has been field-tested and shown to be able to load externals that the old code has not been able to load.
however, you are of course right that the use of "_close()" is indeed an oversight, which only did not trigger a problem so far, as sys_close() is nowhere used yet.
s_path.c: In function ‘sys_open’: s_path.c:450: warning: ‘mode_t’ is promoted to ‘int’ when passed through ‘...’ s_path.c:450: warning: (so you should pass ‘int’ not ‘mode_t’ to ‘va_arg’) s_path.c:450: note: if this code is reached, the program will abort
the patch includes some comments pointing to an online discussion of the problem. to summarize: using mode_t in va_list will always trigger some problems. either we accept the warning (the code will never be reached, since a runtime-test will use a va_arg(..., int) instead) or we move the test to configure (autoconf).
since i am the only one who seems to like autoconf, i decided for the less invasive solution.
I think it makes sense to restore sys_close() for backwards ABI compatibility. Microsoft says that the POSIX close() was deprecated in 2005, and to use their ISO C++ _close() instead [1][2], so the new sys_close() should look like this:
/* close a previously opened file this is needed on platforms where you cannot open/close resources across dll-boundaries */ int sys_close(int fd) { #ifdef _WIN32 return _close(fd); #else return close(fd); #endif }
And leave sys_open, sys_fopen, and sys_fclose as macros in the header. This implementation of sys_open and warning are much more complicated. And tho normally I think its good to avoid #ifdefs in headers, in this case I think it actually communicates why we have these sys_open/sys_close sys_fopen/sys_fclose in the first place: "Win32 lacks good POSIX API support, everything else works as is".
Attached is my patch that I think should replace 2b8a4c13904f6b8bef3a8ae52b5258a131eb6a19 "provide sys_close(),... on all platforms" .hc
[1] http://msdn.microsoft.com/en-US/library/ms235443(v=vs.80).aspx [2] http://msdn.microsoft.com/en-US/library/5fzwd5ss(v=vs.80).aspx
(I attached the patch for reference, it doesn't seem to be up on the patch tracker any more.)
it seems that the patch has moved to the "bug" tracker. i moved it back to "patches" [1].
fgmasdr IOhannes
[1] https://sourceforge.net/tracker/?func=detail&aid=3596865&group_id=55...
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
... but if POSIX has a close() I think there's no issue here - MSW is POSIX compliant, they say, and hence they're committeed to maintaining close(). So I think it's fine just to use close() and not have a sys_close() at all (or if someone is actually using sys_close() we choud just:
int sys_close(int fd) { return close(fd); }
:) M
On Tue, Dec 18, 2012 at 12:22:29PM -0500, Hans-Christoph Steiner wrote:
On Dec 18, 2012, at 4:56 AM, IOhannes m zmölnig wrote:
On 12/18/2012 04:40, Hans-Christoph Steiner wrote:
I think this approach works.
thanks
The patch you provided seems totally untested, as in not even compiled on GNU/Linux or Mac OS X. It includes the _close() function in sys_close() which only works on Win32 and it gives this warning when building on Mac OS X:
thanks for the compliments :-)
i can assure you that the code is tested as in "compiled without warning on gcc-4.7.2 on a debian/gnu linux (wheezy/sid) system" and has been field-tested and shown to be able to load externals that the old code has not been able to load.
however, you are of course right that the use of "_close()" is indeed an oversight, which only did not trigger a problem so far, as sys_close() is nowhere used yet.
s_path.c: In function ‘sys_open’: s_path.c:450: warning: ‘mode_t’ is promoted to ‘int’ when passed through ‘...’ s_path.c:450: warning: (so you should pass ‘int’ not ‘mode_t’ to ‘va_arg’) s_path.c:450: note: if this code is reached, the program will abort
the patch includes some comments pointing to an online discussion of the problem. to summarize: using mode_t in va_list will always trigger some problems. either we accept the warning (the code will never be reached, since a runtime-test will use a va_arg(..., int) instead) or we move the test to configure (autoconf).
since i am the only one who seems to like autoconf, i decided for the less invasive solution.
I think it makes sense to restore sys_close() for backwards ABI compatibility. Microsoft says that the POSIX close() was deprecated in 2005, and to use their ISO C++ _close() instead [1][2], so the new sys_close() should look like this:
/* close a previously opened file this is needed on platforms where you cannot open/close resources across dll-boundaries */ int sys_close(int fd) { #ifdef _WIN32 return _close(fd); #else return close(fd); #endif }
And leave sys_open, sys_fopen, and sys_fclose as macros in the header. This implementation of sys_open and warning are much more complicated. And tho normally I think its good to avoid #ifdefs in headers, in this case I think it actually communicates why we have these sys_open/sys_close sys_fopen/sys_fclose in the first place: "Win32 lacks good POSIX API support, everything else works as is".
Attached is my patch that I think should replace 2b8a4c13904f6b8bef3a8ae52b5258a131eb6a19 "provide sys_close(),... on all platforms" .hc
[1] http://msdn.microsoft.com/en-US/library/ms235443(v=vs.80).aspx [2] http://msdn.microsoft.com/en-US/library/5fzwd5ss(v=vs.80).aspx
(I attached the patch for reference, it doesn't seem to be up on the patch tracker any more.)
it seems that the patch has moved to the "bug" tracker. i moved it back to "patches" [1].
fgmasdr IOhannes
[1] https://sourceforge.net/tracker/?func=detail&aid=3596865&group_id=55...
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
Windows is most definitely not POSIX compliant. If it was, we wouldn't be having this discussion since the Win32 open() would just work. It has lots POSIX compliant things, but is also missing many key ones. For example, WIN32 does not define any of the POSIX open() flags (O_CREAT, O_TRUNC, etc.) but instead uses _O_CREAT, _O_TRUNC, etc. The WIN32 open() permissions flags its uses work differently than POSIX.
Microsoft marked close() as deprecated in 2005. It seems worthwhile to heed that.
The other issue with this patch is the giant warning it gives on Mac OS X: s_path.c: In function ‘sys_open’: s_path.c:456: warning: ‘mode_t’ is promoted to ‘int’ when passed through ‘...’ s_path.c:456: warning: (so you should pass ‘int’ not ‘mode_t’ to ‘va_arg’) s_path.c:456: note: if this code is reached, the program will abort
.hc
On Dec 18, 2012, at 12:28 PM, Miller Puckette wrote:
... but if POSIX has a close() I think there's no issue here - MSW is POSIX compliant, they say, and hence they're committeed to maintaining close(). So I think it's fine just to use close() and not have a sys_close() at all (or if someone is actually using sys_close() we choud just:
int sys_close(int fd) { return close(fd); }
:) M
On Tue, Dec 18, 2012 at 12:22:29PM -0500, Hans-Christoph Steiner wrote:
On Dec 18, 2012, at 4:56 AM, IOhannes m zmölnig wrote:
On 12/18/2012 04:40, Hans-Christoph Steiner wrote:
I think this approach works.
thanks
The patch you provided seems totally untested, as in not even compiled on GNU/Linux or Mac OS X. It includes the _close() function in sys_close() which only works on Win32 and it gives this warning when building on Mac OS X:
thanks for the compliments :-)
i can assure you that the code is tested as in "compiled without warning on gcc-4.7.2 on a debian/gnu linux (wheezy/sid) system" and has been field-tested and shown to be able to load externals that the old code has not been able to load.
however, you are of course right that the use of "_close()" is indeed an oversight, which only did not trigger a problem so far, as sys_close() is nowhere used yet.
s_path.c: In function ‘sys_open’: s_path.c:450: warning: ‘mode_t’ is promoted to ‘int’ when passed through ‘...’ s_path.c:450: warning: (so you should pass ‘int’ not ‘mode_t’ to ‘va_arg’) s_path.c:450: note: if this code is reached, the program will abort
the patch includes some comments pointing to an online discussion of the problem. to summarize: using mode_t in va_list will always trigger some problems. either we accept the warning (the code will never be reached, since a runtime-test will use a va_arg(..., int) instead) or we move the test to configure (autoconf).
since i am the only one who seems to like autoconf, i decided for the less invasive solution.
I think it makes sense to restore sys_close() for backwards ABI compatibility. Microsoft says that the POSIX close() was deprecated in 2005, and to use their ISO C++ _close() instead [1][2], so the new sys_close() should look like this:
/* close a previously opened file this is needed on platforms where you cannot open/close resources across dll-boundaries */ int sys_close(int fd) { #ifdef _WIN32 return _close(fd); #else return close(fd); #endif }
And leave sys_open, sys_fopen, and sys_fclose as macros in the header. This implementation of sys_open and warning are much more complicated. And tho normally I think its good to avoid #ifdefs in headers, in this case I think it actually communicates why we have these sys_open/sys_close sys_fopen/sys_fclose in the first place: "Win32 lacks good POSIX API support, everything else works as is".
Attached is my patch that I think should replace 2b8a4c13904f6b8bef3a8ae52b5258a131eb6a19 "provide sys_close(),... on all platforms" .hc
[1] http://msdn.microsoft.com/en-US/library/ms235443(v=vs.80).aspx [2] http://msdn.microsoft.com/en-US/library/5fzwd5ss(v=vs.80).aspx
(I attached the patch for reference, it doesn't seem to be up on the patch tracker any more.)
it seems that the patch has moved to the "bug" tracker. i moved it back to "patches" [1].
fgmasdr IOhannes
[1] https://sourceforge.net/tracker/?func=detail&aid=3596865&group_id=55...
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
OK.. so how's this:
For close I'll just edit in your sys_close hack :)
For sys_open, let's just unconditionally say:
int imode = va_arg (ap, int); mode=(mode_t)imode;
without the surrounding if(sizeof(mode_t) < sizeof(int)).
I'll hack that in by hand and push to see how it floats :0
M
On Tue, Dec 18, 2012 at 12:35:07PM -0500, Hans-Christoph Steiner wrote:
Windows is most definitely not POSIX compliant. If it was, we wouldn't be having this discussion since the Win32 open() would just work. It has lots POSIX compliant things, but is also missing many key ones. For example, WIN32 does not define any of the POSIX open() flags (O_CREAT, O_TRUNC, etc.) but instead uses _O_CREAT, _O_TRUNC, etc. The WIN32 open() permissions flags its uses work differently than POSIX.
Microsoft marked close() as deprecated in 2005. It seems worthwhile to heed that.
The other issue with this patch is the giant warning it gives on Mac OS X: s_path.c: In function ‘sys_open’: s_path.c:456: warning: ‘mode_t’ is promoted to ‘int’ when passed through ‘...’ s_path.c:456: warning: (so you should pass ‘int’ not ‘mode_t’ to ‘va_arg’) s_path.c:456: note: if this code is reached, the program will abort
.hc
On Dec 18, 2012, at 12:28 PM, Miller Puckette wrote:
... but if POSIX has a close() I think there's no issue here - MSW is POSIX compliant, they say, and hence they're committeed to maintaining close(). So I think it's fine just to use close() and not have a sys_close() at all (or if someone is actually using sys_close() we choud just:
int sys_close(int fd) { return close(fd); }
:) M
On Tue, Dec 18, 2012 at 12:22:29PM -0500, Hans-Christoph Steiner wrote:
On Dec 18, 2012, at 4:56 AM, IOhannes m zmölnig wrote:
On 12/18/2012 04:40, Hans-Christoph Steiner wrote:
I think this approach works.
thanks
The patch you provided seems totally untested, as in not even compiled on GNU/Linux or Mac OS X. It includes the _close() function in sys_close() which only works on Win32 and it gives this warning when building on Mac OS X:
thanks for the compliments :-)
i can assure you that the code is tested as in "compiled without warning on gcc-4.7.2 on a debian/gnu linux (wheezy/sid) system" and has been field-tested and shown to be able to load externals that the old code has not been able to load.
however, you are of course right that the use of "_close()" is indeed an oversight, which only did not trigger a problem so far, as sys_close() is nowhere used yet.
s_path.c: In function ‘sys_open’: s_path.c:450: warning: ‘mode_t’ is promoted to ‘int’ when passed through ‘...’ s_path.c:450: warning: (so you should pass ‘int’ not ‘mode_t’ to ‘va_arg’) s_path.c:450: note: if this code is reached, the program will abort
the patch includes some comments pointing to an online discussion of the problem. to summarize: using mode_t in va_list will always trigger some problems. either we accept the warning (the code will never be reached, since a runtime-test will use a va_arg(..., int) instead) or we move the test to configure (autoconf).
since i am the only one who seems to like autoconf, i decided for the less invasive solution.
I think it makes sense to restore sys_close() for backwards ABI compatibility. Microsoft says that the POSIX close() was deprecated in 2005, and to use their ISO C++ _close() instead [1][2], so the new sys_close() should look like this:
/* close a previously opened file this is needed on platforms where you cannot open/close resources across dll-boundaries */ int sys_close(int fd) { #ifdef _WIN32 return _close(fd); #else return close(fd); #endif }
And leave sys_open, sys_fopen, and sys_fclose as macros in the header. This implementation of sys_open and warning are much more complicated. And tho normally I think its good to avoid #ifdefs in headers, in this case I think it actually communicates why we have these sys_open/sys_close sys_fopen/sys_fclose in the first place: "Win32 lacks good POSIX API support, everything else works as is".
Attached is my patch that I think should replace 2b8a4c13904f6b8bef3a8ae52b5258a131eb6a19 "provide sys_close(),... on all platforms" .hc
[1] http://msdn.microsoft.com/en-US/library/ms235443(v=vs.80).aspx [2] http://msdn.microsoft.com/en-US/library/5fzwd5ss(v=vs.80).aspx
(I attached the patch for reference, it doesn't seem to be up on the patch tracker any more.)
it seems that the patch has moved to the "bug" tracker. i moved it back to "patches" [1].
fgmasdr IOhannes
[1] https://sourceforge.net/tracker/?func=detail&aid=3596865&group_id=55...
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 2012-12-18 18:44, Miller Puckette wrote:
OK.. so how's this:
For close I'll just edit in your sys_close hack :)
good.
For sys_open, let's just unconditionally say:
int imode = va_arg (ap, int); mode=(mode_t)imode;
without the surrounding if(sizeof(mode_t) < sizeof(int)).
afaik, mode_t can be wider than int on some architectures (solaris-64bit?), which would then probably break. since Pd doesn't support solaris right now, we could simply not care about that until we actually meet an architecture that makes problems, and then take counteractions.
the simplest solution would probably be:
int sys_open(const char *path, int oflag, mode_t mode) { int i, fd; char pathbuf[MAXPDSTRING]; sys_bashfilename(path, pathbuf); return open(pathbuf, oflag, mode); }
fgmasdr IOhannes
OK... my suggestion would be just to try not to worry about Solaris :)
M
On Tue, Dec 18, 2012 at 06:54:29PM +0100, IOhannes m zmoelnig wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 2012-12-18 18:44, Miller Puckette wrote:
OK.. so how's this:
For close I'll just edit in your sys_close hack :)
good.
For sys_open, let's just unconditionally say:
int imode = va_arg (ap, int); mode=(mode_t)imode;
without the surrounding if(sizeof(mode_t) < sizeof(int)).
afaik, mode_t can be wider than int on some architectures (solaris-64bit?), which would then probably break. since Pd doesn't support solaris right now, we could simply not care about that until we actually meet an architecture that makes problems, and then take counteractions.
the simplest solution would probably be:
int sys_open(const char *path, int oflag, mode_t mode) { int i, fd; char pathbuf[MAXPDSTRING]; sys_bashfilename(path, pathbuf); return open(pathbuf, oflag, mode); }
fgmasdr IOhannes -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDQrdIACgkQkX2Xpv6ydvQEBwCeMAC+zvRksCo7wJa6+saQzQga WGUAnR67nczaIAqr0UPN+fczzY7jFtaY =XXB7 -----END PGP SIGNATURE-----
Yeah, let's skip Solaris support. The current version you just pushed works for me.
.hc
On Dec 18, 2012, at 12:57 PM, Miller Puckette wrote:
OK... my suggestion would be just to try not to worry about Solaris :)
M
On Tue, Dec 18, 2012 at 06:54:29PM +0100, IOhannes m zmoelnig wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 2012-12-18 18:44, Miller Puckette wrote:
OK.. so how's this:
For close I'll just edit in your sys_close hack :)
good.
For sys_open, let's just unconditionally say:
int imode = va_arg (ap, int); mode=(mode_t)imode;
without the surrounding if(sizeof(mode_t) < sizeof(int)).
afaik, mode_t can be wider than int on some architectures (solaris-64bit?), which would then probably break. since Pd doesn't support solaris right now, we could simply not care about that until we actually meet an architecture that makes problems, and then take counteractions.
the simplest solution would probably be:
int sys_open(const char *path, int oflag, mode_t mode) { int i, fd; char pathbuf[MAXPDSTRING]; sys_bashfilename(path, pathbuf); return open(pathbuf, oflag, mode); }
fgmasdr IOhannes -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDQrdIACgkQkX2Xpv6ydvQEBwCeMAC+zvRksCo7wJa6+saQzQga WGUAnR67nczaIAqr0UPN+fczzY7jFtaY =XXB7 -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 2012-12-18 18:22, Hans-Christoph Steiner wrote:
/* close a previously opened file this is needed on platforms where you cannot open/close resources across dll-boundaries */ int sys_close(int fd) { #ifdef _WIN32 return _close(fd); #else return close(fd); #endif }
yes, that's how i would do it.
And leave sys_open, sys_fopen, and sys_fclose as macros in the header. This implementation of sys_open and warning are much more complicated. And tho normally I think its good to avoid #ifdefs in headers, in this case I think it actually communicates why we have these sys_open/sys_close sys_fopen/sys_fclose in the first place: "Win32 lacks good POSIX API support, everything else works as is".
if somebody had to implement sys_fopen() and friends, then i would understand why you want to minimize the effort, and replace it with ifdef's. but check the code: somebody already has implemented sys_fopen(),... so there is no reason to save labour time.
there could be a very simple way to implement sys_open() without all the vararg headache. make it:
int sys_open(const char*name, int flags, mode_t mode);
that is, force the users to always supply the mode. this would make the header much cleaner (it's clear which arguments need to be present).
this way, sys_open() would not be 100% compatible with POSIX open(). but afaik the reason for the vararg hell is really, that POSIX decided to introduce an extra argument to open() at some later point, and did not want to add yet another function. open() will happily ignore the 3rd argument if it doesn't need it.
fgmadrs IOhannes
try out what I just pushed which is probably no worse than Hannes's..
cheers M
On Tue, Dec 18, 2012 at 06:42:17PM +0100, IOhannes m zmoelnig wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 2012-12-18 18:22, Hans-Christoph Steiner wrote:
/* close a previously opened file this is needed on platforms where you cannot open/close resources across dll-boundaries */ int sys_close(int fd) { #ifdef _WIN32 return _close(fd); #else return close(fd); #endif }
yes, that's how i would do it.
And leave sys_open, sys_fopen, and sys_fclose as macros in the header. This implementation of sys_open and warning are much more complicated. And tho normally I think its good to avoid #ifdefs in headers, in this case I think it actually communicates why we have these sys_open/sys_close sys_fopen/sys_fclose in the first place: "Win32 lacks good POSIX API support, everything else works as is".
if somebody had to implement sys_fopen() and friends, then i would understand why you want to minimize the effort, and replace it with ifdef's. but check the code: somebody already has implemented sys_fopen(),... so there is no reason to save labour time.
there could be a very simple way to implement sys_open() without all the vararg headache. make it:
int sys_open(const char*name, int flags, mode_t mode);
that is, force the users to always supply the mode. this would make the header much cleaner (it's clear which arguments need to be present).
this way, sys_open() would not be 100% compatible with POSIX open(). but afaik the reason for the vararg hell is really, that POSIX decided to introduce an extra argument to open() at some later point, and did not want to add yet another function. open() will happily ignore the 3rd argument if it doesn't need it.
fgmadrs IOhannes
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDQqvUACgkQkX2Xpv6ydvQMoACgj6qsuUMpGwPkBatyC0Tvf9UF lm8Anio5wbFfG0WjPb4CcDU0fgnmaF6z =bTtX -----END PGP SIGNATURE-----
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
-
Hans-Christoph Steiner -
IOhannes m zmoelnig -
IOhannes m zmölnig
-
Miller Puckette