Re: CVS commit: src/lib/libc/stdlib

2010-09-30 Thread Matthias Scheler
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/


Re: CVS commit: src

2010-09-30 Thread Joerg Sonnenberger
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

2010-09-30 Thread Nicolas Joly
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/lib/libc/stdlib

2010-09-30 Thread Matthias Scheler
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

2010-09-30 Thread Nicolas Joly
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

2010-09-30 Thread Christos Zoulas
In article 20100930133211.gc25...@britannica.bec.de,
Joerg Sonnenberger  jo...@britannica.bec.de 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

2010-09-30 Thread Christos Zoulas
In article 20100930134213.ga18...@colwyn.zhadum.org.uk,
Matthias Scheler  t...@netbsd.org 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