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

Attachment: 0001-restore-sys_close-as-a-function-on-all-platforms-to-.patch
Description: Binary data


[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

Reply via email to