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 */ /*