Discussion:
Out of date comment in setenv.c?
(too old to reply)
Reuben Thomas
2017-02-10 12:10:47 UTC
Permalink
Raw Message
In setenv.c, it says

/* This function is used by 'setenv' and 'putenv'. The difference between
the two functions is that for the former must create a new string which
is then placed in the environment, while the argument of 'putenv'
must be used directly. This is all complicated by the fact that we try
to reuse values once generated for a 'setenv' call since we can never
free the strings. */
int
__add_to_environ (const char *name, const char *value, const char *combined,
int replace)

However, I can't see that this function is called anywhere else
(specifically, not in putenv.c), nor that it ever did (but maybe it's
indirect?).

In short, could the text "and 'putenv'" be removed or clarified, please?
--
http://rrt.sc3d.org
Bruno Haible
2017-02-10 18:17:58 UTC
Permalink
Raw Message
Hi Reuben,
Post by Reuben Thomas
In setenv.c, it says
/* This function is used by 'setenv' and 'putenv'. The difference between
the two functions is that for the former must create a new string which
is then placed in the environment, while the argument of 'putenv'
must be used directly. This is all complicated by the fact that we try
to reuse values once generated for a 'setenv' call since we can never
free the strings. */
int
__add_to_environ (const char *name, const char *value, const char *combined,
int replace)
However, I can't see that this function is called anywhere else
(specifically, not in putenv.c), nor that it ever did (but maybe it's
indirect?).
In short, could the text "and 'putenv'" be removed or clarified, please?
setenv.c comes from glibc (file glibc/stdlib/setenv.c). You could guess it
by seeing the _LIBC conditional, or by looking at config/srclist.txt.

In glibc, glibc/stdlib/putenv.c does make use of __add_to_environ. So the
comment is valid in the context of glibc.

In gnulib, we generally try to avoid gratuitous differences to glibc sources,
in order to ease the burden of the occasional re-sync. Therefore I would not
like to change this comment here.

Bruno
Reuben Thomas
2017-02-10 18:20:34 UTC
Permalink
Raw Message
Post by Bruno Haible
setenv.c comes from glibc (file glibc/stdlib/setenv.c). You could guess it
by seeing the _LIBC conditional, or by looking at config/srclist.txt.
​I did wonder about that, but I thought maybe glibc used some code from
gnulib.​

In gnulib, we generally try to avoid gratuitous differences to glibc
Post by Bruno Haible
sources,
in order to ease the burden of the occasional re-sync. Therefore I would not
like to change this comment here.
​Maybe it would be worth adding a banner to say "imported from glibc, do
not touch"?
--
http://rrt.sc3d.org
Bruno Haible
2017-02-10 21:04:36 UTC
Permalink
Raw Message
Post by Bruno Haible
In gnulib, we generally try to avoid gratuitous differences to glibc sources,
in order to ease the burden of the occasional re-sync. Therefore I would not
like to change this comment here.
​Maybe it would be worth adding a banner to say "imported from glibc, do
not touch"?
I'll leave it to Paul to decide about this.

Bruno
Paul Eggert
2017-02-10 21:15:49 UTC
Permalink
Raw Message
Post by Bruno Haible
.
​Maybe it would be worth adding a banner to say "imported from glibc, do
not touch"?
I'll leave it to Paul to decide about this.
We have touched the file, so it might be worth adding a comment that
could in principle be migrated back to glibc.

Loading...