On Fri, 17 Oct 2003, Martin Stjernholm, Roxen IS @ Pike developers forum wrote:
This pop is too early - envvar might point to freed memory after it since it only points to the data block of the string without adding a reference to it.
I saw exactly this error in many places in the Caudium C module. Did you start from that code, or is there some other bad examples out there that people are copying?
Actually, i wrote it from scratch based on looking at other functions in sytem.c. It makes sense to me that would be a problem. I assume then that it is more proper to wait until the latest possible moment before popping items from the stack, right?
Note that pike strings can contain NUL chars, so if you pass them directly to a libc function like this it might read a truncated string. Maybe you should check that the passed string doesn't contain NUL?
what would be the best way to handle strings with NUL? Should they be changed or should an error be thrown? My guess is that an error should be thrown as you really wouldn't want them in the environment, right?
Most functions of this type return 0 on failure and nonzero on success, so maybe you should invert the return value.
Or even better would be to declare putenv as returning void and throw an error instead, since the only possible error is ENOMEM, and pike generally throws errors when running out of memory.
Also, wouldn't it be nicer to let the function take the name and the value in separate strings, so that the user doesn't have to glue together one with "="? Then you could also check that the name doesn't contain "=" already.
Actually, I like the idea of throwing an error on failure. If I were able to get this to work properly, i was thinking about modifying the predef::putenv() to call the System.putenv() if it existed.
Bill