bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: Feature Branch Windows Build Broken - lib/glob.c & WINDOWS32


From: Conrad T. Pino
Subject: RE: Feature Branch Windows Build Broken - lib/glob.c & WINDOWS32
Date: Sat, 28 May 2005 14:41:53 -0700

Hi Derek,

> From: Derek Price
> 
> Conrad T. Pino wrote:
> 
> >Yes, the Windows build completes.  Can you explain what "home_dir" value
> >will be in the "WINDOWS32" undefined logic?
> 
> It looks like it would call, basically, getpwnam (getlogin()).  There
> are substitutes for both in windows-NT/pwd.c, though it doesn't look
> like they are doing much useful.

I'd like to see a general improvement.

> >How about defining "WINDOWS32" and using the patch below?
> 
> Maybe this would be preferable.  This would enable some of the [a-z]:/
> swithcing other places in the file as well.  Of course, it's possible
> much of that could be abstracted out using the FILE_SYSTEM_PREFIX_LEN &
> ISSLASH macros.

I don't have a preference at the moment and I agree with macros comment.

> My feeling is that it might be nice to move as much of your patch as
> possible into pwd.c, abstract out as much of the drive letter prefix
> stuff as possible into the aformentioned macros, skip defining
> WINDOWS32, and see how things look, but it might be a bit of work.  If
> you'd rather take the easy way out, I won't argue, but the former might
> inadvertantly fix a few other hitherto unnoticed or ignored bugs on Windows.

I propose we do both, I'll edit and test, you coach:

I'm OK with leaving WINDOWS32 undefined in our project.  I'd like to see
an improvement in the WINDOWS32 code for the benefit of other projects if
for no other reason than saying thank you to GLib and GNULib by sending
back some useful code.

I will improve "pwd.c" also so long as cutting and pasting is acceptable
between "pwd.c" and "glob.c" is acceptable.

The bit of work concerns me only if we have an open issue that breaks the
Windows build.  If the Windows build works I'll work on this issue.

I assume "inadvertantly fix...former" is the "pwd.c" improvement.  If so
I share your motivation and agree.

> >The #if logic was cut from "lib/glob.c" lines 53 through 71 with the
> >extras discarded.  My patch is a quick hack.  I would appreciate it if
> >you choose to make improvements.
> 
> No, I think what you did is fine, at least until more than one #ifdef is
> needed.  The version in glob.c is nice for making the rest of the
> dirent/direct switching pretty transparent, but it's not needed yet.

Thank you.

> >Index: lib/glob.c
> >===================================================================
> >RCS file: /cvs/ccvs/lib/glob.c,v
> >retrieving revision 1.15
> >diff -u -p -r1.15 glob.c
> >--- lib/glob.c       25 May 2005 20:23:38 -0000      1.15
> >+++ lib/glob.c       26 May 2005 10:57:36 -0000
> >@@ -524,8 +525,45 @@ glob (const char *pattern, int flags,
> >         home_dir = "SYS:";
> > # else
> > #  ifdef WINDOWS32
> >+      /* Windows doesn't set HOME, honor it if user sets it */
> >       if (home_dir == NULL || home_dir[0] == '\0')
> >-            home_dir = "c:/users/default"; /* poor default */
> >+      {
> >+              /* Windows sets USERPROFILE like UNIX sets HOME */
> >+              home_dir = getenv( "USERPROFILE" );
> >+      }
> >+      if (home_dir == NULL || home_dir[0] == '\0')
> >+      {
> >+              /* Windows sets APPDATA to "$USERPROFILE/Application Data" */
> >+              home_dir = getenv( "APPDATA" );
> >+      }
> 
> Do you really expect this to be set if %USERPROFILE% is not?  It depends
> on it...

Windows NT, 2000, XP and 2003 will set the values.  I expect trouble only when
the user has used command window to undefine or redefine which is unlikely.

> >+      if (home_dir == NULL || home_dir[0] == '\0')
> >+      {
> >+              /* Windows sets ALLUSERSPROFILE to "$USERPROFILE/../All 
> >Users" */
> >+              home_dir = getenv( "ALLUSERSPROFILE" );
> >+      }
> 
> Does this really depend on %USERPROFILE% too or is this just your
> shorthand to refer to the user-profile directory?

No "ALLUSERSPROFILE" does not depend on "USERPROFILE" and you are correct on
it's use as shorthand to illustrate they typically have a common root.

Power users to modify the registry can relocate their profile away from the
default profile root.

Networked users with file server resident profiles is another case.

> >+      if (home_dir == NULL || home_dir[0] == '\0')
> >+      {
> >+              /* Windows sets SystemRoot to installation value typically
> >+                     C:/WinNT but frequently C:/Windows */
> >+              /* This may be a bad idea but it's an alternative to the root 
> >*/
> >+              home_dir = getenv( "SystemRoot" );
> >+      }
> 
> I'm not sure how I feel about using SystemRoot as a fallback for ~ or
> ~username...  sounds unintuitive and possibly dangerous.

Windows has no support for "~" and similar.  We can provide it as a reference
to "$HOME", "$USERPROFILE" and/or "$HOMEDRIVE", "$HOMEPATH" combination.

> >+      if (home_dir == NULL || home_dir[0] == '\0')
> >+      {
> >+              /*    "$SystemDrive/users" is Windows NT 4 specific
> >+
> >+                            NEW INSTALLS of Windows 2000 and later use
> >+                            "$SystemDrive/Documents and Settings"
> >+
> >+                            UPGRADES use previous location
> >+
> >+                            The default user profile can't be found with an 
> >environment
> >+                            variable.  It's location is in the Windows 
> >registry.
> >+
> >+                            The SystemDrive environment variable is an 
> >alternative.
> >+                            */
> >+              home_dir = "c:/users/default"; /* poor default */
> >+      }
> 
> CVS already uses %HOMEDRIVE%%HOMPATH% on windows (set automatically for
> a long time on NT and carried through to XP), which sounds like another
> good possibility.  I think it might predate %USERPROFILE%, so
> %USERPROFILE% should probably be checked first.  It might be nice to
> roll this all together into one place, like the pwd.c stuff, to avoid
> having different portions of hte program do different things on Windows
> with respect to $HOME.  Then the get_homedir() function from
> windows-NT/filesubr.c should just check %HOME% then make the getpwname
> (getlogin()) call.

I can't say which came first but it's not particularly relevant to me.

I suggest we continue CVS past practice in a backwards compatible fashion.

I was aware of "HOMEDRIVE" and "HOMEPATH" when I drafted the patch but
didn't include them because I don't know how to cleanly deal with the
allocation and disposal issues the concatenation requires.  Typical
NT and derivative systems will have:

        %HOMEDRIVE%%HOMEPATH% == %USERPROFILE%

I expect the above identity to be broken only if the users has done so.

Since CVS documents "HOME", "HOMEDRIVE" and "HOMEPATH", we should give
these priority over my suggested addition which include "USERPROFILE".
=======================================================================
I assume my suggestion to use the registry is not needed based upon the
lack of feedback and the use of native Win32 API calls likely to create
resistance from GLib and GNULib.

I deliberately ignored Windows 95, 98 and Me when writing the drasft as
I have not worked with any of these systems in years.  I don't care but
will include them consist with CVS Project policy whatever that may be.
Can you share an opinion here?

> Regards,

Ditto,

> Derek

Conrad





reply via email to

[Prev in Thread] Current Thread [Next in Thread]