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=55736&atid=478070
> >>>
> >>> _______________________________________________
> >>> Pd-dev mailing list
> >>> [email protected]
> >>> http://lists.puredata.info/listinfo/pd-dev
> >>
> >
> >> _______________________________________________
> >> Pd-dev mailing list
> >> [email protected]
> >> http://lists.puredata.info/listinfo/pd-dev
> >
>
_______________________________________________
Pd-dev mailing list
[email protected]
http://lists.puredata.info/listinfo/pd-dev
