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
>>> 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
>
_______________________________________________
Pd-dev mailing list
Pd-dev@iem.at
http://lists.puredata.info/listinfo/pd-dev
