Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-09-16 Thread Christoph Otto

chromatic via RT wrote:

On Monday 15 September 2008 23:21:26 Christoph Otto wrote:


--- src/pmc/os.pmc  (revision 31173)
+++ src/pmc/os.pmc  (working copy)
@@ -31,9 +31,6 @@

 #include parrot/parrot.h

-/* XXX Check if we need to deallocate strerror strings */
-/* XXX apparently, strerror_r is thread-safe and should be used instead.*/
-
 static PMC *OS_PMC;
 pmclass OS singleton {

@@ -92,9 +89,8 @@
 RETURN(STRING *scwd);
 }
 else {
-const char * const errmsg = strerror(errno);
 Parrot_ex_throw_from_c_args(interp, NULL,
EXCEPTION_EXTERNAL_ERROR, -errmsg);
+%Ss, Parrot_strerror(interp, errno));
 }
 }


This pattern's almost common enough to tempt me to write 
Parrot_ex_throw_strerror().


-- c




It was also almost enough to make me write Parrot_strerror.  This patch 
contains a file that I neglected to include with the previous one.  Thanks to 
Andy Dougherty for catching that.
Index: src/ops/sys.ops
===
--- src/ops/sys.ops	(revision 31174)
+++ src/ops/sys.ops	(working copy)
@@ -66,13 +66,11 @@
 }
 
 op err(out STR) {
-  const char * const tmp = strerror(errno);
-  $1 = string_make(interp, tmp, strlen(tmp), ascii, 0);
+  $1 = Parrot_strerror(interp, errno);
 }
 
 op err(out STR, in INT) {
-  const char * const tmp = strerror($2);
-  $1 = string_make(interp, tmp, strlen(tmp), ascii, 0);
+  $1 = Parrot_strerror(interp, $2);
 }
 
 
Index: src/pmc/os.pmc
===
--- src/pmc/os.pmc	(revision 31174)
+++ src/pmc/os.pmc	(working copy)
@@ -31,9 +31,6 @@
 
 #include parrot/parrot.h
 
-/* XXX Check if we need to deallocate strerror strings */
-/* XXX apparently, strerror_r is thread-safe and should be used instead.*/
-
 static PMC *OS_PMC;
 pmclass OS singleton {
 
@@ -92,9 +89,8 @@
 RETURN(STRING *scwd);
 }
 else {
-const char * const errmsg = strerror(errno);
 Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+%Ss, Parrot_strerror(interp, errno));
 }
 }
 
@@ -118,9 +114,8 @@
 #endif
 string_cstring_free(cpath);
 if (error) {
-char *errmsg = strerror(errno);
 Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+%Ss, Parrot_strerror(interp, errno));
 }
 }
 
@@ -141,10 +136,9 @@
 int   error = stat(cpath, info);
 
 if (error) {
-const char * const errmsg = strerror(errno);
 string_cstring_free(cpath);
 Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+%Ss, Parrot_strerror(interp, errno));
 }
 
 if (S_ISDIR(info.st_mode)) {
@@ -155,18 +149,16 @@
 #endif
 string_cstring_free(cpath);
 if (error) {
-const char * const errmsg = strerror(errno);
 Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+%Ss, Parrot_strerror(interp, errno));
 }
 }
 else {
 error = remove(cpath);
 string_cstring_free(cpath);
 if (error) {
-const char * const errmsg = strerror(errno);
 Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+%Ss, Parrot_strerror(interp, errno));
 }
 }
 }
@@ -192,9 +184,8 @@
 #endif
 string_cstring_free(cpath);
 if (error) {
-const char * const errmsg = strerror(errno);
 Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+%Ss, Parrot_strerror(interp, errno));
 }
 }
 
@@ -231,9 +222,8 @@
 string_cstring_free(cpath);
 
 if (error) {
-const char * const errmsg = strerror(errno);
 Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+%Ss, Parrot_strerror(interp, errno));
 }
 else {
 PMC * const array = pmc_new(interp, enum_class_FixedPMCArray);
@@ -296,9 +286,8 @@
 string_cstring_free(cpath);
 
 if (error) {
-const char * const errmsg = strerror(errno);
 Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+%Ss, Parrot_strerror(interp, errno));
 }
 else {
 PMC * const array = pmc_new(interp, enum_class_FixedPMCArray);
@@ -343,9 +332,8 @@
 string_cstring_free(cto);
 
 if (error) {
-const char * const errmsg = strerror(errno);
 

Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-09-16 Thread Andy Dougherty
I think you've gone about this in the right way, but need to handle the 
various cases a bit more broadly.  Specifically, you can't assume that 
everyone has strerror() at all (Solaris 8 doesn't, for example) nor that 
everyone who uses int strerror_r() will also define either _XOPEN_SOURCE 
or _POSIX_C_SOURCE (NetBSD doesn't, for example).

I think perhaps a structure like the following might work:

#if defined(HAS_STRERROR_R)
#  if defined(STRERROR_R_RETURNS_INT)
/* stuff using int strerror_r(); */
#  else
/* stuff using char *strerror_r() */
#else
/* stuff using plain strerror(). */
#endif

Currently, you are trying to guess which branch based on various #defines, 
but non-Linux systems will fall through to the wrong branch, and systems 
without strerror_r will fail completely.

Ideally, Configure.pl ought to figure which branch you need. Alas, it 
doesn't.  However, perl5's Configure does (at least for Unix systems), so 
you could just use it's results.

-- 
Andy Dougherty  [EMAIL PROTECTED]


Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-09-13 Thread Christoph Otto
After looking around Parrot and realizing that there were several other places 
where strerror is used, I'm trying a different approach.  The attached patch 
adds config/gen/platform/generic/strerror.c, which contains a new function 
Parrot_strerror.  This is just a wrapper to abstract away the non-reentrancy 
and platform-specific semantics of strerror.  After a reconfigure, Parrot 
builds successfully and all tests pass on Ubuntu/x86.


I would like comments on whether the patch puts its code in the correct places 
and if it's a generally good idea.  If both these are true, I'll go ahead 
testing on other platforms and converting other instances of strerror.


Christoph
Index: src/pmc/file.pmc
===
--- src/pmc/file.pmc	(revision 31046)
+++ src/pmc/file.pmc	(working copy)
@@ -22,9 +22,15 @@
 #  include direct.h
 #endif
 
+#define ERRBUF_SIZE 128
+
 #include parrot/parrot.h
 
-/* RT#46681 apparently, strerror_r is thread-safe and should be used instead.*/
+#  define THROW_FILE_EXCEPTION(interp)  \
+{ \
+STRING *errstr = Parrot_strerror((interp), errno); \
+Parrot_ex_throw_from_c_args((interp), NULL, EXCEPTION_LIBRARY_ERROR, %Ss, errstr); \
+}
 
 static PMC *File_PMC;
 pmclass File singleton {
@@ -73,9 +79,14 @@
 #endif
 string_cstring_free(cpath);
 
-if (error)
+if (error  errno == ENOENT) {
 RETURN(INTVAL 0);
+}
+else if (error) {
+THROW_FILE_EXCEPTION(INTERP);
+}
 
+
 RETURN(INTVAL 1);
 }
 
@@ -89,6 +100,7 @@
 
 */
 
+
 METHOD is_dir(STRING *path) {
 struct stat info;
 char *cpath = string_to_cstring(interp, path);
@@ -100,9 +112,7 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+THROW_FILE_EXCEPTION(INTERP);
 }
 
 if (S_ISDIR(info.st_mode))
@@ -132,9 +142,7 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+THROW_FILE_EXCEPTION(INTERP);
 }
 
 if (S_ISREG(info.st_mode))
@@ -155,20 +163,20 @@
 
 METHOD is_link(STRING *path) {
 #ifdef WIN32
-/* I love win32 implementations */
+/* I love win32. */
 RETURN(INTVAL 0);
 #else
 struct stat info;
+char *cpath;
+int error;
 
-char *cpath = string_to_cstring(interp, path);
-int error   = lstat(cpath, info);
+cpath = string_to_cstring(interp, path);
+error = lstat(cpath, info);
 
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+THROW_FILE_EXCEPTION(INTERP);
 }
 
 if (S_ISLNK(info.st_mode))
@@ -197,11 +205,13 @@
 METHOD copy(STRING *from, STRING *to) {
 #define CHUNK_SIZE 1024
 
+char errmsg[ERRBUF_SIZE];
 char *cfrom  = string_to_cstring(interp, from);
 FILE *source = fopen(cfrom, rb);
 
 string_cstring_free(cfrom);
 
+
 if (source) {
 char *cto= string_to_cstring(interp, to);
 FILE *target = fopen(cto, w+b);
@@ -226,16 +236,12 @@
 fclose(target);
 }
 else {
-char *errmsg = strerror(errno);
-Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+THROW_FILE_EXCEPTION(INTERP);
 }
 fclose(source);
 }
 else {
-char *errmsg = strerror(errno);
-Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+THROW_FILE_EXCEPTION(INTERP);
 }
 #undef CHUNK_SIZE
 }
@@ -259,9 +265,7 @@
 string_cstring_free(cto);
 
 if (error) {
-char *errmsg = strerror(errno);
-Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+THROW_FILE_EXCEPTION(INTERP);
 }
 }
 }
Index: config/gen/platform/win32/string.h
===
--- config/gen/platform/win32/string.h	(revision 31046)
+++ config/gen/platform/win32/string.h	(working copy)
@@ -12,6 +12,8 @@
 #  if _MSC_VER = 1400
 #define strdup _strdup
 #  endif
+/* This is only useful with msvc.  gcc/mingw can use strerror_r. */
+#  define strerror_r(errnum, buf, buflen) strerror_s((buf), (buflen), (errnum))
 #endif
 
 #endif /* PARROT_PLATFORM_WIN32_STRING_H_GUARD */
Index: config/gen/platform/platform_interface.h

Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-09-11 Thread Christoph Otto

Will Coleda via RT wrote:

On Tue Jul 22 23:34:13 2008, [EMAIL PROTECTED] wrote:

Christoph Otto via RT wrote:

This version of the patch should dtrt with all versions of

strerror_r.  It

works on my Debian/x86 box and I'll be testing it on any *nix I can

get my

hands on Tuesday.  If it works fine there, if someone can test it on

windows

and if the patch looks OK, I'll commit it and close this ticket.

Christoph


This patch contains a fix and a simplification.  It should now be
cross-platform and thread-safe.  I'll test on some other *nixes and go
on from
there.  If nothing else it works fine on Ubuntu/x86.


Based on Christoph's instructions in #parrot, I tested this using an MSVC compiler, with the 
following results:

snip explodey brokenness



Here's v8 of the patch.  After poking around on gcc/win, msvc and gcc/linux 
I'm pretty sure this will at least pass all tests on all those platforms and 
anything else that has a sufficiently POSIXy strerror_r.  Unfortunately it 
seems that strerror_s (MS' strerror_r) isn't available to gcc on windows, so 
that platform may be stuck with a non-thread-safe file PMC.  An updated patch 
proving me wrong would be welcome.


It'd be good to get this patch reviewed before it's applied, but the main goal 
is to get all tests passing on Linux/gcc, msvc and win/gcc.
Index: src/pmc/file.pmc
===
--- src/pmc/file.pmc	(revision 30979)
+++ src/pmc/file.pmc	(working copy)
@@ -24,8 +24,56 @@
 
 #include parrot/parrot.h
 
-/* RT#46681 apparently, strerror_r is thread-safe and should be used instead.*/
+/* strerror_r should truncate the message if it's too long for the supplied
+ * buffer.  It's probably best to just specify a sane default buffer size than
+ * to worry about retrying calls. */
+#define ERRBUF_SIZE 128
 
+/* use POSIXy strerror_r (*nix) or strerror_s macro (msvc)*/
+#if (!defined(WIN32)  !defined(_GNU_SOURCE)) || \
+(defined(_MSC_VER)  defined(WIN32))
+#  define THROW_FILE_EXCEPTION(interp, func)  \
+{ \
+char errmsg[ERRBUF_SIZE];   \
+int err_status; \
+err_status = strerror_r(errno, errmsg, ERRBUF_SIZE);  \
+/* Linux's POSIXy strerror_r returns -1 on error, others return an error code */ \
+if (err_status == -1)  \
+err_status = errno;   \
+\
+if (err_status == 0 || err_status == ERANGE) { \
+Parrot_ex_throw_from_c_args(\
+(interp), NULL, EXCEPTION_LIBRARY_ERROR, %s, errmsg); \
+}   \
+else if (err_status == EINVAL){  \
+Parrot_ex_throw_from_c_args((interp), NULL, EXCEPTION_LIBRARY_ERROR, \
+%s returned an invalid error code (%d), #func, errno); \
+}   \
+else {  \
+Parrot_ex_throw_from_c_args((interp), NULL, EXCEPTION_LIBRARY_ERROR, \
+strerror_r() returned an unknown error code: %d, err_status); \
+}   \
+}
+
+/* use GNU-specific strerror_r */
+#elif defined(_GNU_SOURCE)
+#  define THROW_FILE_EXCEPTION(interp, func)  \
+{ \
+/* GNU strerror_r DTRT for unknown error codes */   \
+char errmsg[ERRBUF_SIZE];   \
+char *errstr = strerror_r(errno, errmsg, ERRBUF_SIZE); \
+Parrot_ex_throw_from_c_args((interp), NULL, EXCEPTION_LIBRARY_ERROR, %s, errstr); \
+}
+
+/* using gcc on windows, so we're stuck with plain strerror */
+#else
+#  define THROW_FILE_EXCEPTION(interp, func) \
+{ \
+char *errmsg = strerror(errno); \
+Parrot_ex_throw_from_c_args((interp), NULL, EXCEPTION_LIBRARY_ERROR, %s,errmsg); \
+}
+#endif
+
 static PMC *File_PMC;
 pmclass File singleton {
 
@@ -73,9 +121,18 @@
 #endif
 string_cstring_free(cpath);
 
-if (error)
+if (error  errno == ENOENT) {
 RETURN(INTVAL 0);
+}
+else if (error) {
+#ifdef WIN32
+THROW_FILE_EXCEPTION(INTERP, stat);
+#else
+THROW_FILE_EXCEPTION(INTERP, lstat);
+#endif
+}
 
+
 RETURN(INTVAL 1);
 }
 
@@ -89,6 +146,7 @@
 
 */
 
+
 METHOD is_dir(STRING *path) {
 struct stat info;
 char *cpath = string_to_cstring(interp, path);
@@ -100,9 +158,11 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+#ifdef WIN32
+THROW_FILE_EXCEPTION(INTERP, stat);
+#else
+THROW_FILE_EXCEPTION(INTERP, lstat);
+#endif
 }
 
 if (S_ISDIR(info.st_mode))
@@ -132,9 +192,11 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_EXTERNAL_ERROR,
-errmsg);
+#ifdef WIN32
+THROW_FILE_EXCEPTION(INTERP, stat);
+#else
+THROW_FILE_EXCEPTION(INTERP, 

Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-23 Thread Christoph Otto

Christoph Otto via RT wrote:


This version of the patch should dtrt with all versions of strerror_r.  It 
works on my Debian/x86 box and I'll be testing it on any *nix I can get my 
hands on Tuesday.  If it works fine there, if someone can test it on windows 
and if the patch looks OK, I'll commit it and close this ticket.


Christoph



This patch contains a fix and a simplification.  It should now be 
cross-platform and thread-safe.  I'll test on some other *nixes and go on from 
there.  If nothing else it works fine on Ubuntu/x86.
Index: src/pmc/file.pmc
===
--- src/pmc/file.pmc	(revision 29691)
+++ src/pmc/file.pmc	(working copy)
@@ -24,8 +24,50 @@
 
 #include parrot/parrot.h
 
-/* RT#46681 apparently, strerror_r is thread-safe and should be used instead.*/
+/* strerror_r should truncate the message if it's too long for the supplied
+ * buffer.  It's probably best to just specify a sane default buffer size than
+ * to worry about retrying calls. */
+#define ERRBUF_SIZE 128
 
+#ifndef _GNU_SOURCE
+/* use POSIXy strerror_r */
+#  define STRERROR_R_EXCEPTION(interp, func)  \
+{ \
+char errmsg[ERRBUF_SIZE];   \
+int err_status; \
+err_status = strerror_r(errno, errmsg, ERRBUF_SIZE);  \
+/* Linux's POSIXy strerror_r returns -1 on error, others return an error code */ \
+if (err_status == -1)  \
+err_status = errno;   \
+\
+if (err_status == 0 || err_status == ERANGE) { \
+STRING *errmsg_pstring = string_make((interp), errmsg, strlen(errmsg), NULL, 0);  \
+real_exception((interp), NULL, E_SystemError, %Ss, errmsg_pstring); \
+}   \
+else if (err_status == EINVAL){  \
+real_exception((interp), NULL, E_SystemError, \
+%s returned an invalid error code (%d), #func, errno); \
+}   \
+else {  \
+real_exception((interp), NULL, E_SystemError, \
+strerror_r() returned an unknown error code: %d, err_status); \
+}   \
+}
+#else
+/* use GNU-specific strerror_r */
+#  define STRERROR_R_EXCEPTION(interp, func)  \
+{ \
+/* GNU strerror_r DTRT for unknown error codes */   \
+char errmsg[ERRBUF_SIZE];   \
+char *errstr = strerror_r(errno, errmsg, ERRBUF_SIZE);  \
+STRING *errmsg_pstring = string_make((interp), errstr, strlen(errstr), NULL, 0);  \
+real_exception((interp), NULL, E_SystemError, %Ss, errmsg_pstring); \
+}
+#endif
+
+
+
+
 static PMC *File_PMC;
 pmclass File singleton {
 
@@ -89,6 +131,7 @@
 
 */
 
+
 METHOD is_dir(STRING *path) {
 struct stat info;
 char *cpath = string_to_cstring(interp, path);
@@ -100,8 +143,11 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+#ifdef WIN32
+STRERROR_R_EXCEPTION(INTERP, stat);
+#else
+STRERROR_R_EXCEPTION(INTERP, lstat);
+#endif
 }
 
 if (S_ISDIR(info.st_mode))
@@ -131,8 +177,11 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+#ifdef WIN32
+STRERROR_R_EXCEPTION(INTERP, stat);
+#else
+STRERROR_R_EXCEPTION(INTERP, lstat);
+#endif
 }
 
 if (S_ISREG(info.st_mode))
@@ -164,8 +213,7 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION(INTERP, lstat);
 }
 
 if (S_ISLNK(info.st_mode))
@@ -199,6 +247,8 @@
 
 string_cstring_free(cfrom);
 
+char errmsg[ERRBUF_SIZE];
+
 if (source) {
 char *cto= string_to_cstring(interp, to);
 FILE *target = fopen(cto, w+b);
@@ -223,14 +273,12 @@
 fclose(target);
 }
 else {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION(INTERP, fopen);
 }
 fclose(source);
 }
 else {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION(INTERP, fopen);
 }
 #undef CHUNK_SIZE
 }
@@ -254,8 +302,7 @@
 string_cstring_free(cto);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION(INTERP, rename);
 }
 }
 }
Index: config/gen/platform/win32/string.h
===
--- config/gen/platform/win32/string.h	(revision 29691)
+++ 

[perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-23 Thread Will Coleda via RT
On Tue Jul 22 23:34:13 2008, [EMAIL PROTECTED] wrote:
 Christoph Otto via RT wrote:
 
  This version of the patch should dtrt with all versions of
 strerror_r.  It
  works on my Debian/x86 box and I'll be testing it on any *nix I can
 get my
  hands on Tuesday.  If it works fine there, if someone can test it on
 windows
  and if the patch looks OK, I'll commit it and close this ticket.
 
  Christoph
 
 
 This patch contains a fix and a simplification.  It should now be
 cross-platform and thread-safe.  I'll test on some other *nixes and go
 on from
 there.  If nothing else it works fine on Ubuntu/x86.

Based on Christoph's instructions in #parrot, I tested this using an MSVC 
compiler, with the 
following results:

C:\research\parrotnmake

Microsoft (R) Program Maintenance Utility Version 8.00.50727.42
Copyright (C) Microsoft Corporation.  All rights reserved.

Compiling with:
xx.c
cl -I.\include -nologo -GF -W4 -MD -Zi -DNDEBUG -DWIN32 -D_CONSOLE -DNO_STRICT -
DUSE_SITECUSTOMIZE -DPRIVLIB_LAST_IN_INC -D_CRT_SECURE_NO_DEPRECATE -
DHASATTRIBU
TE_DEPRECATED -wd4101 -DHASATTRIBUTE_NORETURN -wd4101 -Zi -wd4127 -wd4054 -
wd431
0 -DHAS_JIT -DI386 -I. -Fo xx.obj -c xx.c
C:\Perl\bin\perl.exe tools\build\pmc2c.pl --dump src\pmc\file.pmc
C:\Perl\bin\perl.exe tools\build\pmc2c.pl --c src\pmc\file.pmc
C:\Perl\bin\perl.exe tools\build\c2str.pl src\pmc\file.c  src\pmc\file.
str
C:\Perl\bin\perl.exe tools\build\c2str.pl --all
src\string.c
src\pmc\file.c
.\src\pmc\file.pmc(169) : warning C4189: 'pmc' : local variable is initialized b
ut not referenced
.\src\pmc\file.pmc(147) : warning C4013: 'strerror_r' undefined; assuming extern
 returning int
.\src\pmc\file.pmc(318) : warning C4189: 'pmc' : local variable is initialized b
ut not referenced
.\src\pmc\file.pmc(475) : warning C4189: 'pmc' : local variable is initialized b
ut not referenced
.\src\pmc\file.pmc(632) : warning C4189: 'pmc' : local variable is initialized b
ut not referenced
.\src\pmc\file.pmc(633) : warning C4189: 'path' : local variable is initialized
but not referenced
.\src\pmc\file.pmc(250) : error C2143: syntax error : missing ';' before 'type'
NMAKE : fatal error U1077: 'C:\Perl\bin\perl.exe' : return code '0x2'
Stop.

-- 
Will Coke Coleda


[perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-23 Thread Christoph Otto via RT
On Tue Jul 22 23:34:13 2008, [EMAIL PROTECTED] wrote:
 
 This patch contains a fix and a simplification.  It should now be
 cross-platform and thread-safe.  I'll test on some other *nixes and go
 on from
 there.  If nothing else it works fine on Ubuntu/x86.

It also works in FreeBSD 7.0 and OpenBSD 4.3.  If someone can confirm
that it's OK on windows (msvc or cygwin) and that the patch otherwise
looks good, it can be committed and this ticket resolved.


Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-22 Thread Christoph Otto

Andy Dougherty via RT wrote:

On Thu, 17 Jul 2008, Christoph Otto via RT wrote:


On Thu Jul 17 15:53:12 2008, julianalbo wrote:

On Thu, Jul 17, 2008 at 9:59 PM, Christoph Otto via RT
[EMAIL PROTECTED] wrote:


trick.  The attached patch (v5) properly fixes the problem on my system.
 There shouldn't be any remaining issues, but the patch ought to be
tested on a another *nix and Windows.


 [ . . . ]
 char *errstr = strerror_r(errno, errmsg, ERRBUF_SIZE);  


Unfortunately, that's not portable.  The POSIXy version of strerror_r 
returns an integer, specifically 0 if successful, and something else 
otherwise.  (What that something else actually is seems to be 
defined differently by different vendors, alas.  OpenBSD and Solaris 
return errno.  Linux/glibc returns -1.)




Thanks for pointing that out.  After what it's taking to get this to work, I'm 
looking forward to my next patch being silently ignored.  I suppose there's a 
reason my spell checker wanted to turn strerror to terrorist.


This version of the patch should dtrt with all versions of strerror_r.  It 
works on my Debian/x86 box and I'll be testing it on any *nix I can get my 
hands on Tuesday.  If it works fine there, if someone can test it on windows 
and if the patch looks OK, I'll commit it and close this ticket.


Christoph
Index: src/pmc/file.pmc
===
--- src/pmc/file.pmc	(revision 29667)
+++ src/pmc/file.pmc	(working copy)
@@ -24,8 +24,50 @@
 
 #include parrot/parrot.h
 
-/* RT#46681 apparently, strerror_r is thread-safe and should be used instead.*/
+/* strerror_r should truncate the message if it's too long for the supplied
+ * buffer.  It's probably best to just specify a sane default buffer size than
+ * to worry about retrying calls. */
+#define ERRBUF_SIZE 128
 
+/* feature test macro taken from Ubuntu's strerror man page */
+#if (_POSIX_C_SOURCE = 200112L || _XOPEN_SOURCE = 600)  ! _GNU_SOURCE
+/* use POSIXy strerror_r */
+#  define STRERROR_R_EXCEPTION(interp, func)  \
+{ \
+char errmsg[ERRBUF_SIZE];   \
+int err_status; \
+err_status = strerror_r(errno, errmsg, ERRBUF_SIZE);  \
+/* Linux's POSIXy strerror_r returns -1 on error, others return an error code */ \
+if (err_status == -1)  \
+err_status = errno;   \
+\
+if (err_status == 0 || err_status == ERANGE) { \
+STRING *errmsg_pstring = string_make((interp), errmsg, strlen(errmsg), NULL, 0);  \
+real_exception((interp), NULL, E_SystemError, %Ss, errmsg_pstring); \
+}   \
+else if (err_status == EINVAL){  \
+real_exception((interp), NULL, E_SystemError, \
+%s returned an invalid error code (%d), #func, errno); \
+else {  \
+real_exception((interp), NULL, E_SystemError, \
+strerror_r() returned an unknown error code: %d, err_status); \
+}   \
+}
+#else
+/* use GNU-specific strerror_r */
+#  define STRERROR_R_EXCEPTION(interp, func)  \
+{ \
+/* GNU strerror_r DTRT for unknown error codes */   \
+char errmsg[ERRBUF_SIZE];   \
+char *errstr = strerror_r(errno, errmsg, ERRBUF_SIZE);  \
+STRING *errmsg_pstring = string_make((interp), errstr, strlen(errstr), NULL, 0);  \
+real_exception((interp), NULL, E_SystemError, %Ss, errmsg_pstring); \
+}
+#endif
+
+
+
+
 static PMC *File_PMC;
 pmclass File singleton {
 
@@ -89,6 +131,7 @@
 
 */
 
+
 METHOD is_dir(STRING *path) {
 struct stat info;
 char *cpath = string_to_cstring(interp, path);
@@ -100,8 +143,11 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+#ifdef WIN32
+STRERROR_R_EXCEPTION(INTERP, stat);
+#else
+STRERROR_R_EXCEPTION(INTERP, lstat);
+#endif
 }
 
 if (S_ISDIR(info.st_mode))
@@ -131,8 +177,11 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+#ifdef WIN32
+STRERROR_R_EXCEPTION(INTERP, stat);
+#else
+STRERROR_R_EXCEPTION(INTERP, lstat);
+#endif
 }
 
 if (S_ISREG(info.st_mode))
@@ -164,8 +213,7 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION(INTERP, lstat);
 }
 
 if (S_ISLNK(info.st_mode))
@@ -199,6 +247,8 @@
 
 string_cstring_free(cfrom);
 
+char errmsg[ERRBUF_SIZE];
+
 if (source) {
 char *cto= string_to_cstring(interp, to);
 FILE *target = fopen(cto, w+b);
@@ -223,14 +273,12 @@
 fclose(target);
 }
 else {
- 

Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-18 Thread chromatic
On Thursday 17 July 2008 15:44:36 NotFound wrote:

 [EMAIL PROTECTED] wrote:
  With this patch, the new tests still pass on Linux/x86.  The patch uses
  STRING-strstart to avoid leaking a malloc'd buffer when throwing an
  exception, which may or may not be considered kosher in this situation.

 Can't you use real_exception(interp, NULL, E_SystemError, %Ss,
 errmsg_pstring); ?

 Using internal details of parrot strings must be avoided.

NotFound's solution is correct; that's the way to go here.

-- c


[perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-18 Thread Christoph Otto via RT
On Thu Jul 17 15:53:12 2008, julianalbo wrote:
 On Thu, Jul 17, 2008 at 9:59 PM, Christoph Otto via RT
 [EMAIL PROTECTED] wrote:
 
  With this patch, the new tests still pass on Linux/x86.  The patch uses
  STRING-strstart to avoid leaking a malloc'd buffer when throwing an
  exception, which may or may not be considered kosher in this situation.
 
 Can't you use real_exception(interp, NULL, E_SystemError, %Ss,
 errmsg_pstring); ?
 
 Using internal details of parrot strings must be avoided.
 

I agree.  I just didn't know real_exception was smart enough to do that
trick.  The attached patch (v5) properly fixes the problem on my system.
 There shouldn't be any remaining issues, but the patch ought to be
tested on a another *nix and Windows.
Index: src/pmc/file.pmc
===
--- src/pmc/file.pmc	(revision 29551)
+++ src/pmc/file.pmc	(working copy)
@@ -26,6 +26,21 @@
 
 /* RT#46681 apparently, strerror_r is thread-safe and should be used instead.*/
 
+/* strerror_r should truncate the message if its too long for the supplied buffer
+ * it's probably best to just specify a sane default buffer size than to worry
+ * about retrying calls
+ */
+#define ERRBUF_SIZE 128
+
+#define STRERROR_R_EXCEPTION()  \
+{ \
+char errmsg[ERRBUF_SIZE];   \
+char *errstr = strerror_r(errno, errmsg, ERRBUF_SIZE);  \
+STRING *errmsg_pstring = string_make(interp, errstr, strlen(errstr), NULL, 0);  \
+real_exception(interp, NULL, E_SystemError, %Ss, errmsg_pstring); \
+}
+
+
 static PMC *File_PMC;
 pmclass File singleton {
 
@@ -89,6 +104,7 @@
 
 */
 
+
 METHOD is_dir(STRING *path) {
 struct stat info;
 char *cpath = string_to_cstring(interp, path);
@@ -100,8 +116,7 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 
 if (S_ISDIR(info.st_mode))
@@ -131,8 +146,7 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 
 if (S_ISREG(info.st_mode))
@@ -164,8 +178,7 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 
 if (S_ISLNK(info.st_mode))
@@ -199,6 +212,8 @@
 
 string_cstring_free(cfrom);
 
+char errmsg[ERRBUF_SIZE];
+
 if (source) {
 char *cto= string_to_cstring(interp, to);
 FILE *target = fopen(cto, w+b);
@@ -223,14 +238,12 @@
 fclose(target);
 }
 else {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 fclose(source);
 }
 else {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 #undef CHUNK_SIZE
 }
@@ -254,8 +267,7 @@
 string_cstring_free(cto);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 }
 }
Index: config/gen/platform/win32/string.h
===
--- config/gen/platform/win32/string.h	(revision 29551)
+++ config/gen/platform/win32/string.h	(working copy)
@@ -14,6 +14,8 @@
 #  endif
 #endif
 
+#define strerror_r(errnum, buf, buflen) strerror_s((buf), (buflen), (errnum))
+
 #endif /* PARROT_PLATFORM_WIN32_STRING_H_GUARD */
 
 /*


Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-18 Thread jerry gay
On Thu, Jul 17, 2008 at 7:44 PM, Christoph Otto via RT
[EMAIL PROTECTED] wrote:
 On Thu Jul 17 15:53:12 2008, julianalbo wrote:
 On Thu, Jul 17, 2008 at 9:59 PM, Christoph Otto via RT
 [EMAIL PROTECTED] wrote:

  With this patch, the new tests still pass on Linux/x86.  The patch uses
  STRING-strstart to avoid leaking a malloc'd buffer when throwing an
  exception, which may or may not be considered kosher in this situation.

 Can't you use real_exception(interp, NULL, E_SystemError, %Ss,
 errmsg_pstring); ?

 Using internal details of parrot strings must be avoided.


 I agree.  I just didn't know real_exception was smart enough to do that
 trick.  The attached patch (v5) properly fixes the problem on my system.
  There shouldn't be any remaining issues, but the patch ought to be
 tested on a another *nix and Windows.

iirc we shouldn't be assuming interp in macros (it should always be passed).
i could be wrong, so i've copied the man with the feathered broom for
an official ruling.
~jerry


Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-18 Thread Andy Dougherty
On Thu, 17 Jul 2008, Christoph Otto via RT wrote:

 On Thu Jul 17 15:53:12 2008, julianalbo wrote:
  On Thu, Jul 17, 2008 at 9:59 PM, Christoph Otto via RT
  [EMAIL PROTECTED] wrote:
  
 trick.  The attached patch (v5) properly fixes the problem on my system.
  There shouldn't be any remaining issues, but the patch ought to be
 tested on a another *nix and Windows.

 [ . . . ]
  char *errstr = strerror_r(errno, errmsg, ERRBUF_SIZE);  

Unfortunately, that's not portable.  The POSIXy version of strerror_r 
returns an integer, specifically 0 if successful, and something else 
otherwise.  (What that something else actually is seems to be 
defined differently by different vendors, alas.  OpenBSD and Solaris 
return errno.  Linux/glibc returns -1.)

-- 
Andy Dougherty  [EMAIL PROTECTED]


[perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-17 Thread Christoph Otto via RT
On Wed Apr 23 18:18:00 2008, [EMAIL PROTECTED] wrote:
 This thread trailed off about 4 months ago.  Could we get an update on
 its status, i.e., whether it should be applied, what OSes it's passing
 on, etc.
 
 Thank you very much.
 kid51

The tests passed because the strerror/strerror_r code doesn't have any
coverage.  I added some tests that make sure the error message consists
only of alphanumerics and whitespace in r29551.  The tests pass with the
current src/pmc/file.pmc, but fail with the attached patch.  I don't
have the tuits to fix the patch, but at least now it's obvious that it
was broken.


[perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-17 Thread Christoph Otto via RT
On Thu Jul 17 01:17:51 2008, cotto wrote:
 On Wed Apr 23 18:18:00 2008, [EMAIL PROTECTED] wrote:
  This thread trailed off about 4 months ago.  Could we get an update on
  its status, i.e., whether it should be applied, what OSes it's passing
  on, etc.
  
  Thank you very much.
  kid51
 
 The tests passed because the strerror/strerror_r code doesn't have any
 coverage.  I added some tests that make sure the error message consists
 only of alphanumerics and whitespace in r29551.  The tests pass with the
 current src/pmc/file.pmc, but fail with the attached patch.  I don't
 have the tuits to fix the patch, but at least now it's obvious that it
 was broken.

With this patch, the new tests still pass on Linux/x86.  The patch uses
STRING-strstart to avoid leaking a malloc'd buffer when throwing an
exception, which may or may not be considered kosher in this situation.

If this is judged appropriate, I'd appreciate test confirmation from at
least OSX/x86 and Windows since strerror_r seems to be a special function.


Index: src/pmc/file.pmc
===
--- src/pmc/file.pmc	(revision 29551)
+++ src/pmc/file.pmc	(working copy)
@@ -26,6 +26,21 @@
 
 /* RT#46681 apparently, strerror_r is thread-safe and should be used instead.*/
 
+/* strerror_r should truncate the message if its too long for the supplied buffer
+ * it's probably best to just specify a sane default buffer size than to worry
+ * about retrying calls
+ */
+#define ERRBUF_SIZE 128
+
+#define STRERROR_R_EXCEPTION()  \
+{ \
+char errmsg[ERRBUF_SIZE];   \
+char *errstr = strerror_r(errno, errmsg, ERRBUF_SIZE);  \
+STRING *errmsg_pstring = string_make(interp, errstr, strlen(errstr), NULL, 0);  \
+real_exception(interp, NULL, E_SystemError, errmsg_pstring-strstart); \
+}
+
+
 static PMC *File_PMC;
 pmclass File singleton {
 
@@ -89,6 +104,7 @@
 
 */
 
+
 METHOD is_dir(STRING *path) {
 struct stat info;
 char *cpath = string_to_cstring(interp, path);
@@ -100,8 +116,7 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 
 if (S_ISDIR(info.st_mode))
@@ -131,8 +146,7 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 
 if (S_ISREG(info.st_mode))
@@ -164,8 +178,7 @@
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 
 if (S_ISLNK(info.st_mode))
@@ -199,6 +212,8 @@
 
 string_cstring_free(cfrom);
 
+char errmsg[ERRBUF_SIZE];
+
 if (source) {
 char *cto= string_to_cstring(interp, to);
 FILE *target = fopen(cto, w+b);
@@ -223,14 +238,12 @@
 fclose(target);
 }
 else {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 fclose(source);
 }
 else {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 #undef CHUNK_SIZE
 }
@@ -254,8 +267,7 @@
 string_cstring_free(cto);
 
 if (error) {
-char *errmsg = strerror(errno);
-real_exception(interp, NULL, E_SystemError, errmsg);
+STRERROR_R_EXCEPTION();
 }
 }
 }
Index: config/gen/platform/win32/string.h
===
--- config/gen/platform/win32/string.h	(revision 29551)
+++ config/gen/platform/win32/string.h	(working copy)
@@ -14,6 +14,8 @@
 #  endif
 #endif
 
+#define strerror_r(errnum, buf, buflen) strerror_s(buf, buflen, errnum)
+
 #endif /* PARROT_PLATFORM_WIN32_STRING_H_GUARD */
 
 /*


Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-07-17 Thread NotFound
On Thu, Jul 17, 2008 at 9:59 PM, Christoph Otto via RT
[EMAIL PROTECTED] wrote:

 With this patch, the new tests still pass on Linux/x86.  The patch uses
 STRING-strstart to avoid leaking a malloc'd buffer when throwing an
 exception, which may or may not be considered kosher in this situation.

Can't you use real_exception(interp, NULL, E_SystemError, %Ss,
errmsg_pstring); ?

Using internal details of parrot strings must be avoided.

-- 
Salu2


[perl #46681] [TODO] [C] Use strerror_r instead of strerror

2008-04-23 Thread James Keenan via RT
This thread trailed off about 4 months ago.  Could we get an update on
its status, i.e., whether it should be applied, what OSes it's passing
on, etc.

Thank you very much.
kid51


Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2007-11-29 Thread Will Coleda


On Nov 28, 2007, at 1:29 AM, Joe Sadusk via RT wrote:


Heh, ok, grammar fixed and resubmitted.  And yes, I've tested it on
Linux (Fedora 7) and Windows with MSVC 2005.  I don't have a non-glibc
unix to try it out with however.

Joe

On Mon Nov 26 09:50:28 2007, [EMAIL PROTECTED] wrote:

On Sunday 25 November 2007 21:48:01 Joe Sadusk via RT wrote:


Ok, here's try two.  Platform specific stuff has been moved into
config/gen/platform, and non-win32 platforms use strerror_r  
directly.


Change its to it's in the comment before #define ERRBUF_SIZE  
and this
patch looks reasonable to me, if it passes on Windows and Unixy  
platforms.


-- c






Tested on osx/x86; all tests pass.


--
Will Coke Coleda
[EMAIL PROTECTED]




[perl #46681] [TODO] [C] Use strerror_r instead of strerror

2007-11-28 Thread Joe Sadusk via RT
Heh, ok, grammar fixed and resubmitted.  And yes, I've tested it on
Linux (Fedora 7) and Windows with MSVC 2005.  I don't have a non-glibc
unix to try it out with however.

Joe

On Mon Nov 26 09:50:28 2007, [EMAIL PROTECTED] wrote:
 On Sunday 25 November 2007 21:48:01 Joe Sadusk via RT wrote:
 
  Ok, here's try two.  Platform specific stuff has been moved into
  config/gen/platform, and non-win32 platforms use strerror_r directly.
 
 Change its to it's in the comment before #define ERRBUF_SIZE and this 
 patch looks reasonable to me, if it passes on Windows and Unixy platforms.
 
 -- c
 





strerror_3.patch
Description: Binary data


[perl #46681] [TODO] [C] Use strerror_r instead of strerror

2007-11-26 Thread Joe Sadusk via RT
Ok, here's try two.  Platform specific stuff has been moved into
config/gen/platform, and non-win32 platforms use strerror_r directly.

On Fri Nov 23 01:29:35 2007, kjs wrote:
 On Nov 22, 2007 9:03 PM, Joe Sadusk via RT
 [EMAIL PROTECTED] wrote:
  Reposting this because this is my first patch, and it didn't occur to me
  that if I don't CC perl6-internals, no one would notice it.
 
  It turns out that strerror_r is POSIX only, and windows has the slightly
  different strerror_s (exactly the same except the arguments are in a
  different order). I defined a macro that abstracts this, but I don't
  know if there's a standard header file cross platform macros like this
  are put in, so I put it inline in the pmc. If anyone has a suggestion of
  where this belongs, I'll gladly move it.
 
 I'm no expert on this, but in config/gen/platform, there are
 subdirectories that contain files that do platform dependent stuff.
 (I only know this because I once added
 config/gen/platform/win32/string.h, which defines a macro for strdup
 for MSVC users.)
 
 I suspect that your macro should go in a similar file, but this should
 be confirmed by someone with more knowledge on this.
 
 kjs
 



diff --git a/config/gen/platform/generic/string.h b/config/gen/platform/generic/string.h
new file mode 100644
index 000..8de1ca1
--- /dev/null
+++ b/config/gen/platform/generic/string.h
@@ -0,0 +1,22 @@
+/*
+ * $Id$
+ * Copyright (C) 2004-2007, The Perl Foundation.
+ */
+
+#ifndef PARROT_PLATFORM_GENERIC_STRING_H_GUARD
+#define PARROT_PLATFORM_GENERIC_STRING_H_GUARD
+
+#if defined __GLIBC__
+#  define _XOPEN_SOURCE 600
+#endif
+
+#include string.h
+
+#endif /* PARROT_PLATFORM_GENERIC_STRING_H_GUARD */
+
+/*
+ * Local variables:
+ *   c-file-style: parrot
+ * End:
+ * vim: expandtab shiftwidth=4:
+ */
diff --git a/config/gen/platform/win32/string.h b/config/gen/platform/win32/string.h
index 607745a..bd47db3 100644
--- a/config/gen/platform/win32/string.h
+++ b/config/gen/platform/win32/string.h
@@ -14,6 +14,8 @@
 #  endif
 #endif
 
+#define strerror_r(errnum, buf, buflen) strerror_s(buf, buflen, errnum)
+
 #endif /* PARROT_PLATFORM_WIN32_STRING_H_GUARD */
 
 /*
diff --git a/src/pmc/file.pmc b/src/pmc/file.pmc
index 3c8d801..56be62d 100644
--- a/src/pmc/file.pmc
+++ b/src/pmc/file.pmc
@@ -27,6 +27,13 @@ CFile is a singleton class which provides access to File functions.
 /* RT#46679 Check if we need to deallocate strerror strings */
 /* RT#46681 apparently, strerror_r is thread-safe and should be used instead.*/
 
+/* strerror_r should truncate the message if its too long for the supplied buffer
+ * its probably best to just specify a sane default buffer size than to worry
+ * about retrying calls
+ */
+#define ERRBUF_SIZE 128
+
+
 static PMC *File_PMC;
 pmclass File singleton {
 
@@ -98,7 +105,8 @@ Returns a true value (1) if the supplied path is a directory.
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
+char errmsg[ERRBUF_SIZE];
+strerror_r(errno, errmsg, ERRBUF_SIZE);
 real_exception(interp, NULL, E_SystemError, errmsg);
 }
 
@@ -129,7 +137,8 @@ Returns a true value (1) if the supplied path is a plain file.
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
+char errmsg[ERRBUF_SIZE];
+strerror_r(errno, errmsg, ERRBUF_SIZE);
 real_exception(interp, NULL, E_SystemError, errmsg);
 }
 
@@ -162,7 +171,8 @@ Returns a true value (1) if the supplied path is a link.
 string_cstring_free(cpath);
 
 if (error) {
-char *errmsg = strerror(errno);
+char errmsg[ERRBUF_SIZE];
+strerror_r(errno, errmsg, ERRBUF_SIZE);
 real_exception(interp, NULL, E_SystemError, errmsg);
 }
 
@@ -197,6 +207,8 @@ free to change or give me hints on how to change it. -- ambs
 
 FILE *source = fopen(cfrom, rb);
 
+char errmsg[ERRBUF_SIZE];
+
 if (source) {
 FILE *target = fopen(cto, w+b);
 
@@ -218,13 +230,13 @@ free to change or give me hints on how to change it. -- ambs
 fclose(target);
 }
 else {
-char *errmsg = strerror(errno);
+strerror_r(errno, errmsg, ERRBUF_SIZE);
 real_exception(interp, NULL, E_SystemError, errmsg);
 }
 fclose(source);
 }
 else {
-char *errmsg = strerror(errno);
+strerror_r(errno, errmsg, ERRBUF_SIZE);
 real_exception(interp, NULL, E_SystemError, errmsg);
 }
 string_cstring_free(cfrom);
@@ -251,7 +263,8 @@ Rename a file Cfrom to the path Cto.
 string_cstring_free(cto);
 
 if (error) {
-char *errmsg = strerror(errno);
+char errmsg[ERRBUF_SIZE];
+strerror_r(errno, errmsg, ERRBUF_SIZE);
 

Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2007-11-26 Thread chromatic
On Sunday 25 November 2007 21:48:01 Joe Sadusk via RT wrote:

 Ok, here's try two.  Platform specific stuff has been moved into
 config/gen/platform, and non-win32 platforms use strerror_r directly.

Change its to it's in the comment before #define ERRBUF_SIZE and this 
patch looks reasonable to me, if it passes on Windows and Unixy platforms.

-- c


Re: [perl #46681] [TODO] [C] Use strerror_r instead of strerror

2007-11-23 Thread Klaas-Jan Stol
On Nov 22, 2007 9:03 PM, Joe Sadusk via RT
[EMAIL PROTECTED] wrote:
 Reposting this because this is my first patch, and it didn't occur to me
 that if I don't CC perl6-internals, no one would notice it.

 It turns out that strerror_r is POSIX only, and windows has the slightly
 different strerror_s (exactly the same except the arguments are in a
 different order). I defined a macro that abstracts this, but I don't
 know if there's a standard header file cross platform macros like this
 are put in, so I put it inline in the pmc. If anyone has a suggestion of
 where this belongs, I'll gladly move it.

I'm no expert on this, but in config/gen/platform, there are
subdirectories that contain files that do platform dependent stuff.
(I only know this because I once added
config/gen/platform/win32/string.h, which defines a macro for strdup
for MSVC users.)

I suspect that your macro should go in a similar file, but this should
be confirmed by someone with more knowledge on this.

kjs


[perl #46681] [TODO] [C] Use strerror_r instead of strerror

2007-11-22 Thread Joe Sadusk via RT
Reposting this because this is my first patch, and it didn't occur to me
that if I don't CC perl6-internals, no one would notice it.

It turns out that strerror_r is POSIX only, and windows has the slightly
different strerror_s (exactly the same except the arguments are in a
different order). I defined a macro that abstracts this, but I don't
know if there's a standard header file cross platform macros like this
are put in, so I put it inline in the pmc. If anyone has a suggestion of
where this belongs, I'll gladly move it.

There is also the case of older UNIX platforms such as Solaris 7, which
decided to take standard strerror and make it thread safe by using
thread local storage.  How much of a priority are platforms like this?


[perl #46681] [TODO] [C] Use strerror_r instead of strerror

2007-10-22 Thread via RT
# New Ticket Created by  Paul Cochrane 
# Please include the string:  [perl #46681]
# in the subject line of all future correspondence about this issue. 
# URL: http://rt.perl.org/rt3/Ticket/Display.html?id=46681 


In src/pmc/file.pmc there is the todo item:

/* XXX apparently, strerror_r is thread-safe and should be used instead.*/

So, convert all instances of strerror to strerror_r and test.