I recently began work on bug 239765, and came across the wonder of FILE_PATHSEP:
#ifdef XP_UNIX
#define FILE_PATHSEP '/' #define FILE_PATHSEPP "/" #define FILE_PARENT "../" #define WSACleanup()
#elif defined(XP_WIN32)
#define FILE_PATHSEP '/' #define FILE_PATHSEPP "\\" #define FILE_PARENT "..\"
#endif /* XP_WIN32 */
Firstly, what little I knew about windows file handling told me that we could, at least on the supply side, use a unix /.
Indeed, I noted that much of create_instance does exactly that *and* passes in FILE_PATHSEP via %c. If we needed FILE_PATHSEP, then this would already be a problem:
/* generate <confdir>/slapd-collations.conf */ PR_snprintf(src, sizeof(src), "%s%c%s%c/config/%s-collations.conf", cf->sysconfdir, FILE_PATHSEP, cf->package_name, FILE_PATHSEP, PRODUCT_NAME); PR_snprintf(dest, sizeof(dest), "%s%c%s-collations.conf", cf->config_dir, FILE_PATHSEP, PRODUCT_NAME);
Even more priceless is the use of FILE_PATHSEP in ldap/admin/src/configure_instance.cpp:create_console_script(). (is there a /bin/sh that doesn't accept / as a path?).
Anyway, for my own amusement, I'll post a patch (probably untested on win32) to make this a little easier on the eyes, and perhaps even easier on new programmers to the codebase.
Thoughts?
Andrew Bartlett
Andrew Bartlett wrote:
I recently began work on bug 239765, and came across the wonder of FILE_PATHSEP:
#ifdef XP_UNIX
#define FILE_PATHSEP '/' #define FILE_PATHSEPP "/" #define FILE_PARENT "../" #define WSACleanup()
#elif defined(XP_WIN32)
#define FILE_PATHSEP '/' #define FILE_PATHSEPP "\\" #define FILE_PARENT "..\"
#endif /* XP_WIN32 */
Firstly, what little I knew about windows file handling told me that we could, at least on the supply side, use a unix /.
Yes. For modern versions of Windows. This may have been an artifact of Win95/98/NT, or perhaps cases where these paths were passed to a command interpreter (e.g. in a system() call).
Indeed, I noted that much of create_instance does exactly that *and* passes in FILE_PATHSEP via %c. If we needed FILE_PATHSEP, then this would already be a problem:
/* generate <confdir>/slapd-collations.conf */ PR_snprintf(src, sizeof(src), "%s%c%s%c/config/%s-collations.conf", cf->sysconfdir, FILE_PATHSEP, cf->package_name, FILE_PATHSEP, PRODUCT_NAME); PR_snprintf(dest, sizeof(dest), "%s%c%s-collations.conf", cf->config_dir, FILE_PATHSEP, PRODUCT_NAME);
It probably doesn't matter anymore now that modern versions of Windows support both \ and /.
Even more priceless is the use of FILE_PATHSEP in ldap/admin/src/configure_instance.cpp:create_console_script(). (is there a /bin/sh that doesn't accept / as a path?).
Probably not anymore, if there ever was one that had a problem.
Anyway, for my own amusement, I'll post a patch (probably untested on win32) to make this a little easier on the eyes, and perhaps even easier on new programmers to the codebase.
Thoughts?
FILE_PATHSEP is used throughout fedora ds code . . .
Andrew Bartlett
-- Fedora-directory-devel mailing list Fedora-directory-devel@redhat.com https://www.redhat.com/mailman/listinfo/fedora-directory-devel
On Mon, 2007-05-21 at 07:49 -0600, Richard Megginson wrote:
Andrew Bartlett wrote:
Anyway, for my own amusement, I'll post a patch (probably untested on win32) to make this a little easier on the eyes, and perhaps even easier on new programmers to the codebase.
Thoughts?
FILE_PATHSEP is used throughout fedora ds code . . .
Yes and no. It seems confined mainly to the admin code (I presume the rest is isolated by NSPR?).
Attached is the patch I made up, for create_instance at least. For the moment, It includes the schema work (because the two overlap), but I'll redo this once the schema work is in the tree.
Some feedback from a windows system would be useful.
Andrew Bartlett
Andrew Bartlett wrote:
I recently began work on bug 239765, and came across the wonder of FILE_PATHSEP:
#ifdef XP_UNIX
#define FILE_PATHSEP '/' #define FILE_PATHSEPP "/" #define FILE_PARENT "../" #define WSACleanup()
#elif defined(XP_WIN32)
#define FILE_PATHSEP '/' #define FILE_PATHSEPP "\\" #define FILE_PARENT "..\"
#endif /* XP_WIN32 */
Firstly, what little I knew about windows file handling told me that we could, at least on the supply side, use a unix /.
Indeed, I noted that much of create_instance does exactly that *and* passes in FILE_PATHSEP via %c. If we needed FILE_PATHSEP, then this would already be a problem:
/* generate <confdir>/slapd-collations.conf */ PR_snprintf(src, sizeof(src), "%s%c%s%c/config/%s-collations.conf", cf->sysconfdir, FILE_PATHSEP, cf->package_name, FILE_PATHSEP, PRODUCT_NAME); PR_snprintf(dest, sizeof(dest), "%s%c%s-collations.conf", cf->config_dir, FILE_PATHSEP, PRODUCT_NAME);Even more priceless is the use of FILE_PATHSEP in ldap/admin/src/configure_instance.cpp:create_console_script(). (is there a /bin/sh that doesn't accept / as a path?).
Anyway, for my own amusement, I'll post a patch (probably untested on win32) to make this a little easier on the eyes, and perhaps even easier on new programmers to the codebase.
Thoughts?
Andrew Bartlett
DS used to include a significant portion Netscape Enterprise Server (NES) 3.0. Prior to open sourcing I removed several hundred unused files and hundreds more lines of unnecessary code. I suspect that this is NES-specific code that found its way into DS in a viral sort of way.
In any case, it is only the 2 P version that is really a problem, right? The code you included would do single-character replacements AFAICT.
Prior to open sourcing I spent more time than I'd like to admit on getting it to at least compile on win32.
On top of this there are separate re-definitions of this code, in configure_instance.cpp for example. That can probably be removed.
Now certainly one shouldn't mix FILE_PATHSEP with an explicit "/" in the same PR_snprintf. I think that there is less effort these days to support win32.
As for create_console_script(), I have a fairly old source tree but in mine the whole function is #if 0'd out and even then is only meant to run on unix-like systems.
Its important to remember that portitions of the DS code supported versions of Solaris back to 2.4, OSF1, Irix, HP/ux, AIX, Linux and Windows (and perhaps one or two more that have slipped my mine). You haven't lived until you've broken the build on 6 platforms :-) On my team the unlucky developer got the attached plungers displayed on top of their cube.
rob
On Mon, 2007-05-21 at 09:50 -0400, Rob Crittenden wrote:
Its important to remember that portitions of the DS code supported versions of Solaris back to 2.4, OSF1, Irix, HP/ux, AIX, Linux and Windows (and perhaps one or two more that have slipped my mine). You haven't lived until you've broken the build on 6 platforms :-) On my team the unlucky developer got the attached plungers displayed on top of their cube.
Yeah, this is why I put a lot of time and effort into Samba's automated testing, and the real-time results on the build farm. We get a lot of value from that resource.
Any news/plans for a public build farm for Fedora DS?
Sadly for samba, the postage on the plungers would be prohibitive ;-)
Andrew Bartlett
Andrew Bartlett wrote:
On Mon, 2007-05-21 at 09:50 -0400, Rob Crittenden wrote:
Its important to remember that portitions of the DS code supported versions of Solaris back to 2.4, OSF1, Irix, HP/ux, AIX, Linux and Windows (and perhaps one or two more that have slipped my mine). You haven't lived until you've broken the build on 6 platforms :-) On my team the unlucky developer got the attached plungers displayed on top of their cube.
Yeah, this is why I put a lot of time and effort into Samba's automated testing, and the real-time results on the build farm. We get a lot of value from that resource.
Any news/plans for a public build farm for Fedora DS?
Yes, there are plans for a tinderbox-style environment (probably using buildbot instead of tinderbox, though). I'll defer to those folks working on it for an update of the progress.
Sadly for samba, the postage on the plungers would be prohibitive ;-)
Andrew Bartlett
-- Fedora-directory-devel mailing list Fedora-directory-devel@redhat.com https://www.redhat.com/mailman/listinfo/fedora-directory-devel
389-devel@lists.fedoraproject.org