Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

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



    Date:        Sat, 4 Apr 2020 21:16:45 +0000
    From:        David Holland <dholland-sourcechanges%netbsd.org@localhost>
    Message-ID:  <20200404211645.GA19194%netbsd.org@localhost>

  | I fail to see any scenario in which it's legitimate for an application
  | to scribble in internal data belonging to libc. Why should this be
  | permitted?

You've never done something like

	p = getenv("PATH");	/* NULL check omitted here */
	while (q = strchr(p, ':')) {
		*q = '\0';
		/* use p to do a lookup, printf, or whatever */
		*q = ':';
		p = q + 1;
	}
	/* use p here too */

??

If you have, what's the difference, genenv() is also returning data
from libc, it could also be const char *, but isn't.

In neither case is this what would normally be called "internal data"
though, it isn't as if we're directly modifying the data structs malloc()
uses (that would be internal data) - these are the results returned from
libc to the application.

  | I don't understand. It is a bug that the library returns a writeable
  | pointer to data that should not be modified.

But we aren't returning a pointer to data that can't be modified,
libc doesn't care, one way or the other.   Apps shouldn't modify it
(shouldn't attempt to) because of portability concerns with other
implementations.    If we did as Joerg suggested and mmapped
the message catalog pages for the relevant locale, and simply returned
a pointer to the relevant entry, one of those could be us.    But isn't.

  | Anyway, this is moot because strerror is defined by C;

That was my point.   The entry in the BUGS section of the man page was
merely a rant, as this isn't (wasn't ever) something that can be fixed.

  | but because it *should* be const,
  | it should be (and is) declared with __aconst in string.h.
  |
  | Please don't change that.

Not planning anything like that - this was all about the man page.

  | Also, perhaps we should swipe the text about not modifyingthe string
  | buffer from strsignal.3.

I believe that what is in strerror(3) now is more or less aligned
with the intent (if not the actual language) of that.

Note that strsignal(3) doesn't contain a BUGS section ranting about
how it should really be const char *strsignal().

kre




Home | Main Index | Thread Index | Old Index