# [PATCH] unix path separators in spl - Thread: https://externals.io/message/38364 - Original archive: https://news-web.php.net/php.internals/38364 - Started by Steph Fox on 2008-06-18T20:28:40+00:00 - Messages: 12 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #38364 — Steph Fox — 2008-06-18T20:28:40+00:00 URL: https://externals.io/message/38364#38364 Hi all, In Phar we have an internal Windows-only function named phar_unixify_path_separators(), which is needed because Phar is intended to be used with web apps. Phar inherits from SPL, which currently uses the system-specific DEFAULT_SLASH when returning directory paths. This isn't actually a problem in Phar (at least, as far as I'm aware) but it does look... well, untidy. The attached patch implements spl_unixify_path_separators(). If it's accepted, we can probably kill the version in Phar and rely on the definition in SPL. However I'm concerned that some other extension might be relying on SPL's current behaviour, unlikely tho' it may seem. I haven't found any change in behaviour running the test suite for: ext/spl, ext/phar, ext/standard/array, ext/simplexml. I haven't tested: ext/pdo, ext/sqlite, ext/mysqli or pecl/spl_types, and I'm not certain if there should be any more on that list. If you find/can see any problems with this change at all, pls adv. If there are no objections, I plan to commit at the end of this week. Thanks, - Steph --- ## #38420 — Stanislav Malyshev — 2008-06-19T17:43:31+00:00 URL: https://externals.io/message/38364#38420 · In reply to: #38364 Hi! > The attached patch implements spl_unixify_path_separators(). If it's I'm afraid that automatically applying slash conversions to all results returned by SPL file functions - if that's what this patch does - may be unwanted. This way SPL functions and regular file functions would return different file names, which can be harmful if these names are used as keys, compared, etc. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #38426 — Steph Fox — 2008-06-19T18:37:13+00:00 URL: https://externals.io/message/38364#38426 · In reply to: #38420 > > […] > > I'm afraid that automatically applying slash conversions to all results > returned by SPL file functions - if that's what this patch does - may be > unwanted. This way SPL functions and regular file functions would return > different file names, which can be harmful if these names are used as > keys, compared, etc. Paths. Mm, possibly. On the other hand, the only code likely to be broken would be platform-specific code written for Windows boxes - how much of that is likely to be out there? Another point is that, although Windows *writes* '\', these days it's capable of recognising either separator. So it really would just be down to path comparisons. I'm actually wondering how much harm it would do to make DEFAULT_SLASH generic... - Steph --- ## #38424 — Stanislav Malyshev — 2008-06-19T18:39:07+00:00 URL: https://externals.io/message/38364#38424 · In reply to: #38426 Hi! > On the other hand, the only code likely to be broken would be > platform-specific code written for Windows boxes - how much of that is > likely to be out there? Another point is that, although Windows *writes* Any amount, but I don't see why it would break only windows-specific code. If I use any other function to get filenames, and/or use `PATH_SEPARATOR` to compose names, and then use these names in the same context as those converting SPL function, I'd get different names on Windows even if my application was written in most generic way. Do I miss something? > '\', these days it's capable of recognising either separator. So it > really would just be down to path comparisons. Path comparison is important. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #38425 — Steph Fox — 2008-06-19T18:54:21+00:00 URL: https://externals.io/message/38364#38425 · In reply to: #38424 > Any amount, but I don't see why it would break only windows-specific > code. If I use any other function to get filenames, and/or use > `PATH_SEPARATOR` to compose names, and then use these names in the same > context as those converting SPL function, I'd get different names on > Windows even if my application was written in most generic way. Do I > miss something? Doesn't `PATH_SEPARATOR` use DEFAULT_SLASH? > > […] > > Path comparison is important. I thought so too until I couldn't find any broken tests. - Steph > -- > Stanislav Malyshev, Zend Software Architect > stas@zend.com http://www.zend.com/ > (408)253-8829 MSN: stas@zend.com --- ## #38428 — Stanislav Malyshev — 2008-06-19T18:58:04+00:00 URL: https://externals.io/message/38364#38428 · In reply to: #38425 Hi! > > […] > > Doesn't `PATH_SEPARATOR` use DEFAULT_SLASH? Should be `DIRECTORY_SEPARATOR` and it's "\" on windows (PATH_SEPARATOR is ";" ). Do you propose to change it to "/"? > > […] > > I thought so too until I couldn't find any broken tests. With all due respect to phpt, these are very primitive code samples testing very narrow functional areas. They do not come anywhere near the stuff people do in real-life apps, combining different functions and extensions. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #38429 — Marcus Boerger — 2008-06-19T19:00:52+00:00 URL: https://externals.io/message/38364#38429 · In reply to: #38364 Hello Steph, how about having this as an option inside the SPL classes that gets turned on by Phar automatically? Inside SPL we could have it as a user set-only flag. marcus Wednesday, June 18, 2008, 10:28:40 PM, you wrote: > Index: ext/spl/spl_directory.c > =================================================================== > RCS file: /repository/php-src/ext/spl/spl_directory.c,v > retrieving revision 1.45.2.27.2.23.2.22 > diff -u -r1.45.2.27.2.23.2.22 spl_directory.c > --- ext/spl/spl_directory.c 18 Jun 2008 10:05:29 -0000 1.45.2.27.2.23.2.22 > +++ ext/spl/spl_directory.c 18 Jun 2008 17:16:39 -0000 > @@ -185,6 +185,9 @@ > intern->file_name_len = > spprintf(&intern->file_name, 0, "%s%c%s", > > spl_filesystem_object_get_path(intern, `NULL` TSRMLS_CC), > DEFAULT_SLASH, intern->u.dir.entry.d_name); > +#ifdef PHP_WIN32 > + spl_unixify_path_separators(intern->file_name, intern->file_name_len); > +#endif > break; > } > } > @@ -1196,6 +1199,9 @@ > subdir->u.dir.sub_path_len = strlen(intern->u.dir.entry.d_name); > subdir->u.dir.sub_path = > estrndup(intern->u.dir.entry.d_name, subdir->u.dir.sub_path_len); > } > +#ifdef PHP_WIN32 > + spl_unixify_path_separators(intern->u.dir.sub_path, intern->u.dir.sub_path_len); > +#endif > subdir->info_class = intern->info_class; > subdir->file_class = intern->file_class; > subdir->oth = intern->oth; > @@ -1227,6 +1233,9 @@ > > if (intern->u.dir.sub_path) { > len = spprintf(&sub_name, 0, "%s%c%s", > intern->u.dir.sub_path, DEFAULT_SLASH, intern->u.dir.entry.d_name); > +#ifdef PHP_WIN32 > + spl_unixify_path_separators(sub_name, len); > +#endif > RETURN_STRINGL(sub_name, len, 0); > } else { > RETURN_STRING(intern->u.dir.entry.d_name, 1); > Index: ext/spl/spl_directory.h > =================================================================== > RCS file: /repository/php-src/ext/spl/spl_directory.h,v > retrieving revision 1.12.2.5.2.4.2.10 > diff -u -r1.12.2.5.2.4.2.10 spl_directory.h > --- ext/spl/spl_directory.h 20 May 2008 21:46:50 -0000 1.12.2.5.2.4.2.10 > +++ ext/spl/spl_directory.h 18 Jun 2008 17:10:18 -0000 > @@ -112,6 +112,20 @@ > return (spl_filesystem_object*)((char*)it - > XtOffsetOf(spl_filesystem_object, it)); > } > > +#ifdef PHP_WIN32 > +static inline void spl_unixify_path_separators(char *path, int path_len) > +{ > + char *s; > + > + /* unixify win paths */ > + for (s = path; s - path < path_len; ++s) { > + if (*s == '\\') { > + *s = '/'; > + } > + } > +} > +#endif > + > #define SPL_FILE_OBJECT_DROP_NEW_LINE 0x00000001 /* drop new lines */ > #define SPL_FILE_OBJECT_READ_AHEAD 0x00000002 /* read on rewind/next */ > #define SPL_FILE_OBJECT_SKIP_EMPTY 0x00000006 /* skip empty lines */ Best regards, Marcus --- ## #38431 — Steph Fox — 2008-06-19T19:10:31+00:00 URL: https://externals.io/message/38364#38431 · In reply to: #38429 Hi Marcus, > how about having this as an option inside the SPL classes that gets > turned on by Phar automatically? Inside SPL we could have it as a user > set-only flag. You are brilliant. I owe you a beer :) - Steph --- ## #38432 — Steph Fox — 2008-06-19T19:13:47+00:00 URL: https://externals.io/message/38364#38432 · In reply to: #38428 Hi Stas, > > […] > > Should be `DIRECTORY_SEPARATOR` Bleh, must be nearly switching-off time > and it's "\" on windows (PATH_SEPARATOR is ";" ). Do you propose to change > it to "/"? dirsep_str[0] = DEFAULT_SLASH; dirsep_str[1] = '\0'; REGISTER_STRING_CONSTANT("DIRECTORY_SEPARATOR", dirsep_str, CONST_CS|CONST_PERSISTENT); > > […] > > With all due respect to phpt, these are very primitive code samples > testing very narrow functional areas. They do not come anywhere near the > stuff people do in real-life apps, combining different functions and > extensions. Fair point. I like Marcus' idea tho'. - Steph > -- > Stanislav Malyshev, Zend Software Architect > stas@zend.com http://www.zend.com/ > (408)253-8829 MSN: stas@zend.com --- ## #38433 — Stanislav Malyshev — 2008-06-19T19:33:35+00:00 URL: https://externals.io/message/38364#38433 · In reply to: #38432 Hi! > Fair point. I like Marcus' idea tho'. As long as the default stays the same and doesn't hurt anybody not touching the option - go for it. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #39245 — Steph Fox — 2008-07-23T20:24:35+00:00 URL: https://externals.io/message/38364#39245 · In reply to: #38429 Hi Marcus, > how about having this as an option inside the SPL classes that gets > turned on by Phar automatically? Inside SPL we could have it as a user > set-only flag. The attached patch does this, at the SPL end. Can I commit it (or something very close) before the 5_3 freeze please? NB The patch was created with DOS line endings and then converted to *nix, it won't apply cleanly in its current state. It's just for review. Thanks, - Steph --- ## #39251 — Marcus Boerger — 2008-07-23T21:47:10+00:00 URL: https://externals.io/message/38364#39251 · In reply to: #39245 Hello Steph, Wednesday, July 23, 2008, 10:24:35 PM, you wrote: > Hi Marcus, > > […] > The attached patch does this, at the SPL end. Can I commit it (or something > very close) before the 5_3 freeze please? It's a fix is it not? anyway, please go ahead and submit. > NB The patch was created with DOS line endings and then converted to *nix, > it won't apply cleanly in its current state. It's just for review. > Thanks, > - Steph Best regards, Marcus