This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: add mkstemps


Eric Blake wrote:
Howland Craig D (Craig <howland <at> LGSInnovations.com> writes:

An alternate thought:
Practically, size_t versus int for the suffix length makes no
difference at all.  That is, both int and size_t can easily have
numbers that are far too big compared to FILENAME_MAX (or any of the
other variations for max path lengths) to actually work.  If size_t
were used, passing a negative integer would come out to be too big and
result in an error.

Therefore, why not declare it to be the "proper" size_t from the start?

That may be fine for 32-bit systems, but causes an ABI change on 64-bit systems if it ever gets standardized as int because of existing practice. The standards tend to favor existing practice, and since libiberty, BSD, and Solaris all used int rather than size_t, that would probably be what gets standardized (along with rules for when to fail with EINVAL).


Thinking of this, a potential weakness in the new code is that it
does not sanity-check the given suffix length. (Unless perhaps I've
missed a touchup, as one of the gmane comments that Eric made implies
that some kind of negative length check was added.)

Yes, my sandbox has another touchup not previously posted on this list, which I had in place before your email. I basically changed the internal helper function to use size_t (but the public mkstemps still uses int), and added an additional comparison for (trv - path < suffixlen) as an early exit case prior to doing trv -= suffixlen.


Please post any updated patches you intend to check-in as it makes it hard to review based on
my low ESP skills :)
Secondly, the addition probably should be in unistd.h. stdlib.h is
properly fully standardized from ISO and POSIX.

stdlib.h can still be fully compliant, with proper #ifdef guards to hide non-
standard functions when the user is requesting strict compliance. I disagree about <unistd.h> for two reasons: 1) <unistd.h> implies standardized functions, but mkstemps is not (yet) standardized; 2) existing practice - Solaris chose <stdlib.h>.


We should avoid
"polluting" it with non-standard items (not that it isn't already).

Again, the pollution should be something that can be controlled by #ifdef switches (similar to glibc's _GNU_SOURCE, or AIX' _ALL_SOURCE, or Solaris' __EXTENSIONS__, etc). True, newlib is not yet at the point where all extensions are properly guarded by a single macro, but that's not an excuse for putting mkstemps in the correct header to begin with.


If it needed to be moved later if it ever became standard, then move it
later. (Or perhaps now is a good time to introduce a stdlibext.h
file, which is one of the things that I've been thinking about as a
possible mechanism for cleaning up the system header files.)

But that's the whole point of this thread - there is existing practice that puts it in <stdlib.h>, so putting it in a non-standard header like <stdlibext.h> is only doing a disservice to code trying to be portable across all the systems that already provide this function as an extension.


If stdlibext.h is included from stdlib.h, then stdlib.h becomes less cluttered. I assume that is what
Craig is suggesting. This does not need to be done as part of this patch.


-- Jeff J.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]