Re: CVS commit: src/lib/libc/stdlib
In article <20100930134213.ga18...@colwyn.zhadum.org.uk>, Matthias Scheler wrote: >On Thu, Sep 30, 2010 at 03:35:41PM +0200, Nicolas Joly wrote: >> That may be part of the problem (at least for zsh 4.2). >> >> http://www.opengroup.org/onlinepubs/009695399/functions/putenv.html >> >> >From the OpenGroup function description; this function does not copy >> the provided string, but use it directly instead. It's the caller >> responsability to clean it when not in use anymore. > >I see. > >> zsh 4.2 seems to follow this and try to deallocate the previous >> variable string when it has been replaced by a new one. But our >> putenv, which calls setenv, has already done the same ... > >So it assumes that it gets back from getenv(3) exactly what it passed >into putenv(3). Well, it seesm our putenv(3) needs a rewrite. >I think that setenv(3) and unsetenv(3) are correct as they are. Well, if you fix putenv, please move the saveenv realloc environ code in __allocenv() so it is not in two places... christos
Re: CVS commit: src
In article <20100930133211.gc25...@britannica.bec.de>, Joerg Sonnenberger wrote: >For the record, I objected this alternative to my earlier patch because >it once again buries the Alpha madness deeply inside MD headers. This is >just asking to recreate the problem the next time someone wants to deal >with the symbol table hash and there is still no real indication of >Alpha being the odd man out. The alternatives are to: - leave it as is - break backwards compatibility in the binaries - handle both formats Which one? christos
Re: CVS commit: src/lib/libc/stdlib
On Thu, Sep 30, 2010 at 12:41:34PM +, Matthias Scheler wrote: > Module Name: src > Committed By: tron > Date: Thu Sep 30 12:41:33 UTC 2010 > > Modified Files: > src/lib/libc/stdlib: setenv.c unsetenv.c > > Log Message: > Be slightly more careful about freeing memory allocated for environment > variables: only free memory if the current value points to the same > memory area as the allocated block. This will prevent crashes if an > application changes the order of the environment array. > > Unfortunately this is still not enough to stop zsh 4.2.* from crashing. > zsh 4.3.* works fine before and after this change. Thanks Matthias, One possibility could be to not free memory at all in setenv, but only with unsetenv. IMHO, it's a programmer error to call setenv more than once on the same variable without a corresponding unsetenv in between (just like malloc()/free() behaviour). Otherwise, if i understand things correctly, it's more putenv that shoud work that way. -- Nicolas Joly Biological Software and Databanks. Institut Pasteur, Paris.
Re: CVS commit: src/lib/libc/stdlib
On Thu, Sep 30, 2010 at 03:35:41PM +0200, Nicolas Joly wrote: > That may be part of the problem (at least for zsh 4.2). > > http://www.opengroup.org/onlinepubs/009695399/functions/putenv.html > > >From the OpenGroup function description; this function does not copy > the provided string, but use it directly instead. It's the caller > responsability to clean it when not in use anymore. I see. > zsh 4.2 seems to follow this and try to deallocate the previous > variable string when it has been replaced by a new one. But our > putenv, which calls setenv, has already done the same ... So it assumes that it gets back from getenv(3) exactly what it passed into putenv(3). Well, it seesm our putenv(3) needs a rewrite. I think that setenv(3) and unsetenv(3) are correct as they are. Kind regards -- Matthias Scheler http://zhadum.org.uk/
Re: CVS commit: src/lib/libc/stdlib
On Thu, Sep 30, 2010 at 02:11:19PM +0100, Matthias Scheler wrote: > On Thu, Sep 30, 2010 at 03:05:26PM +0200, Nicolas Joly wrote: > > One possibility could be to not free memory at all in setenv, but only > > with unsetenv. > > I'm not convinced about that. > > > IMHO, it's a programmer error to call setenv more than once on the > > same variable without a corresponding unsetenv in between (just like > > malloc()/free() behaviour). > > Is there any standard which backs this statement? Not that i know. > > Otherwise, if i understand things > > correctly, it's more putenv that shoud work that way. > > Our putenv(3) is implemented via setenv(3). That may be part of the problem (at least for zsh 4.2). http://www.opengroup.org/onlinepubs/009695399/functions/putenv.html >From the OpenGroup function description; this function does not copy the provided string, but use it directly instead. It's the caller responsability to clean it when not in use anymore. zsh 4.2 seems to follow this and try to deallocate the previous variable string when it has been replaced by a new one. But our putenv, which calls setenv, has already done the same ... -- Nicolas Joly Biological Software and Databanks. Institut Pasteur, Paris.
Re: CVS commit: src
For the record, I objected this alternative to my earlier patch because it once again buries the Alpha madness deeply inside MD headers. This is just asking to recreate the problem the next time someone wants to deal with the symbol table hash and there is still no real indication of Alpha being the odd man out. Joerg On Thu, Sep 30, 2010 at 09:11:19AM +, Nick Hudson wrote: > Module Name: src > Committed By: skrll > Date: Thu Sep 30 09:11:19 UTC 2010 > > Modified Files: > src/libexec/ld.elf_so: headers.c rtld.h > src/libexec/ld.elf_so/arch/alpha: alpha_reloc.c > src/sys/arch/alpha/include: elf_machdep.h > src/sys/sys: exec_elf.h > > Log Message: > Introduce a new type Elf_Symindx for use in decoding the symbol hash table > section and allow this type to be overridden. > > The ELF specification says it should always be uint32_t (Elf_Word), but > alpha decided to be different (not sure why). Define Elf_Symindx to be > uint64_t on alpha. > > Alpha no longer uses non-standard definitions of Elf64_Sword and > Elf64_Word. Remove the ability to override these types. > > Fixes ld.elf_so after Herculean effort from me and martin. > > > To generate a diff of this commit: > cvs rdiff -u -r1.30 -r1.31 src/libexec/ld.elf_so/headers.c > cvs rdiff -u -r1.92 -r1.93 src/libexec/ld.elf_so/rtld.h > cvs rdiff -u -r1.37 -r1.38 src/libexec/ld.elf_so/arch/alpha/alpha_reloc.c > cvs rdiff -u -r1.11 -r1.12 src/sys/arch/alpha/include/elf_machdep.h > cvs rdiff -u -r1.103 -r1.104 src/sys/sys/exec_elf.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files.
Re: CVS commit: src/lib/libc/stdlib
On Thu, Sep 30, 2010 at 03:05:26PM +0200, Nicolas Joly wrote: > One possibility could be to not free memory at all in setenv, but only > with unsetenv. I'm not convinced about that. > IMHO, it's a programmer error to call setenv more than once on the > same variable without a corresponding unsetenv in between (just like > malloc()/free() behaviour). Is there any standard which backs this statement? > Otherwise, if i understand things > correctly, it's more putenv that shoud work that way. Our putenv(3) is implemented via setenv(3). Kind regards -- Matthias Scheler http://zhadum.org.uk/
