Re: GNUstep.sh / env sanity patches

[Sheldon Gill]

> Right on.  I sort of wanted to trigger discussion about it, which has
> happened, more so than demand its immediate overview or anything like
> that.  Patience is definitely a virtue.

Cool, we're agreed.

> > One thing is that it contains big changes to configure, not
> > configure.ac so
> > they'll be lost at the next autoconf...
>
> I believe I included all neccessary configure.ac changes (checking on
> that), and i wasn't sure whether to include the configure diffs in the
> patch.  Other changes when into GNUmakefile.in.  I blame the
> differences generated in configure likely from different versions of
> autoconf or something like that; I really didn't know whether to
> include it or not ... really, "./configure" isn't supposed to be
> included in source packages anyway, is it?  Kind of something that
> just stuck around 'cause it's pretty silly to auto generate it all the
> time, right?

Seemed to me (from quick perusal) that configure.ac changes didn't reflect 
configure changes.

> > Another big thing is the incorrect memory handling w.r.t. environment
> > variables.
>
> How so?  I can fix this if you tell me what was done incorrectly. :-)

Because you asked for it, I've provided an app at the end which contains the 
essence of your make- and base- patches and hopefully explains things for 
you.

> > So basically I'd definitely reject make- and base- because of the
> > bugs and
> > because of architecture changes.
>
> The bugs hopefully can be fixed if I figure out what's wrong, but ...
> well, yeah, it is an architecture change, but I was focusing on not
> changing old behavior: I just need things to _work_ at this point.  I
> mean, the point was a very light, fully backwards compatible
> architecture change, so if it's rejected because of an architecture
> change ... well, I wouldn't know what to say, as that's the point.

I didn't mean you were causing architecture changes. I mean that *I* was 
changing the architecture and those changes mostly seem to invalidate your 
patch.

> > The stepsh-env patch is going to take more digesting.
>
> The patch is (embarrassingly?) simple: one of the scripts only sets
> the env, the other calls make_services and looks for a
> ~/GNUstep/GNUstep.sh (same as the old GNUstep.sh did) .... the new
> GNUstep.sh calls both, for compatability with the old GNUstep.sh.

Yes, I got that. That's not the issue. I've separated things further to make 
it unnecessary to source GNUstep.sh for normal users.  That pushes the 
requirements of make_services up to a more OS level where it belongs. The 
workspace/OS should be responsible for handling registry of applications, 
types, services etc. That's more of the difficulty in digesting your patch.

> >> I'd like to see GNUstep.sh killed because it doesn't work in my
> >> shell.
> >> I don't know what's happening with this, though. When I asked
> >> #gnustep,
> >> "lack of developer time" was thought to be the hold-up.
> >
> > I've got GNUstep.sh killed for users but not for developers.
> > gnustep-make is
> > too heavily dependent on it at the moment to make it immediately
> > disappear.
>
> Oh, this I would love to help test ... hasn't killing it for users
> been the only goal?  I suppose developers are uses too. :-) heh.

Not for me it hasn't. As a developer, too, I no longer need to source it at 
start up. Want to help test? That can be arranged I'm sure...

> P.S. Just to be clear, I hope I'm not coming off as "pissy" or "angry"
> or anything like that.  I, like you, really want to fix this "the
> right way", but this patch, I was hoping, was a good short term
> extension, for my ends, that didn't screw anyone else up.  When you do
> go over it, please let me know the better/correct way to handle memory
> in this case -- it can only serve both our ends for you to elucidate
> my mistake ... betters the moral/coding ecology or something like
> that. ;-)  So, I guess, please just realize that while the problems
> I'm having are very frustrating to me, I'm not trying to push it's
> resolution as a priority on anyone else.  I'd rather people in our
> realm see me as a teammate and not some interloper.

I hear and understand your frustration.  I'd like to point out, though, that I 
did ask you to try and document precisely what build issues were leading you 
to change $HOME etc so that the *issue* could be identified and properly 
tested against.

> P.P.S. I hate string/memory handling in straight C. ;-)

I hear you brother.  It gets much easier if you have a sane library to work 
with.

Now, as promised, some source for you to explain:
------------------<env_test.c>-----------------
#include <stdlib.h>
#include <stdio.h>
#include <strings.h>

int main( int argc, char **argv )
{
  char *env_var;
  char *user;

        env_var = malloc(2048);
        printf("malloc: env_var is '%lx'\n",(long)env_var);
        
        /* the following is wrong because:
         *  currently, env_var is pointing at the memory we've allocated
         *  but we're about to re-assign it a new value
         *  hence we'll lose the pointer to the allocated memory
         *    and so we have a leak
         */
        if((NULL != (env_var = getenv("GNUSTEP_USER_ROOT"))))
          {
                printf("getenv: env_var is '%lx'\n",(long)env_var);

                /* the following is inefficient because:
                 *   we don't need the extra byte allocated for the string
                 *     as it serves no purpose, just wasted space.
                 */
                user = malloc(strlen(env_var) + 1);
        
            /* the following is wrong because:
                 *   we haven't verified that user is a valid pointer yet
                 *     so if the allocation failed we're in deep trouble
                 */
                strcpy(user, env_var);
          }
        /* the following is wrong because:
         *   env_var is now either NULL or contains the address of the
         *     environment variable, *NOT* the allocated memory
         *   so we're about to try to free memory we didn't allocate
         */     
        printf("free: env_var is '%lx'\n",(long)env_var);
        free(env_var);

        /* we shouldn't get here because we'll seg-fault first */
        printf("end of program.\n");
        return 0;
}