5428 provide fts(), reallocarray(), and strtonum()

Review Request #398 — Created March 11, 2017 and submitted


Provide several bsd compat functions and header:
- sys/cdefs.h: for now only containing __BEGIN_DECLS and __END_DECLS -- dropped for the moment
- fts family: similar to ftw(3C), but a bit more sophisticated
- reallocarray(): similar to realloc(3C), but working on arrays
- strtonum(): a safer alternative to atoi(3C) and strtol(3C) families

for the moment:
- build
- new mandoc version

  • 0
  • 0
  • 6
  • 1
  • 7
Description From Last Updated
  1. In general, I think this looks reasonable. I'd recommend that we make sure to do the pkgsrc testing we talked about. In addition, I'd avoid creating yet another directory in libc that we have to know about and just use port/gen if we can. Otherwise, it seems like this should enable some more amount of additional portability.
    1. Yes, will do, just making it looks sane. Thanks for the review!

  2. usr/src/cmd/mdb/common/mdb/mdb_string.h (Diff revision 3)
    Hmm, it looks like this was never part of the actual external mdb api at least. Though it's a bit unfortunate, I guess there isn't really a great option here.
    1. Not sure what to do here (about open issue), so I'll just mark it as dropped.

  3. usr/src/head/fts.h (Diff revision 3)
    Please use the correct illumos header guard, no trailing '_' character.
  4. usr/src/head/fts.h (Diff revision 3)
    I'd recommend keeping this to be like a standard illumos prototype, where all of this is included in the extern "C" bit, not just the functions.
  5. usr/src/lib/libc/amd64/Makefile (Diff revision 3)
    Why create a new directory? Can't we just put them in port/gen?
    1. OK, moved to gen/. I just thought it would actually be useful to introduce another category (e.g. for things coming from stdlib.h, same as in bsds) seeing as gen/ becoming a dump for just about everything.

  6. usr/src/man/man3c/fts.3c (Diff revision 3)
    Just saying safe here actually isn't useful. From my read of the implementation, only one thread should be calling operations on a single instance of an FTS structure at any time. It's worth calling this out.
    I'd strongly recommend that you write something like the locking section in libavl for this.
    1. Will do.

    2. Looks like this was missed in the most recent update?  Would it help if I provided some text?
    3. Yes, I did everything except this item, and it would really help, thanks Robert!

    4. How's something like this:

      > .Ss Locking
      > The fts routines provide no locking. While the
      > .Fn fts_open
      > function is
      > .Sy Safe
      > and can be called from multiple threads simultaneously, the individual
      > handles returned from the
      > .Fn fts_open
      > function are not thread-safe. If callers need to operate on a single
      > .Vt FTS
      > structure, then it is their responsibility to ensure that none of the
      > other functions are called from multiple threads simultaneously. This
      > implies that the
      > .Fn fts_read ,
      > .Fn fts_children ,
      > .Fn fts_set ,
      > and
      > .Fn fts_close
      > functions are
      > .Sy Unsafe .
      > .Pp
      > These routines are not
      > .Sy Async-Signal-Safe
      > and callers should not assume that the implementation of these functions
      > will be
      > .Sy Fork-Safe .
      > If callers implement their own locking structures around the use of
      > these routines, they must ensure that those locks are accounted for when
      > forking by the use of routines such as
      > .Xr pthread_atfork 3C .
      < .Sy Safe .
      > See
      > .Sx Locking .
      < .Xr qsort 3C
      > .Xr qsort 3C ,
      > .Xr attributes 5
    5. Thanks, added!

  7. usr/src/man/man3c/malloc.3c (Diff revision 3)
    Missing reallocarray up here?
  8. usr/src/man/man3c/malloc.3c (Diff revision 3)
    Or reallocarray?
  1. Thanks for the follow ups here, Yuri. I'm happy being a reviewer on this.

Review request changed

Status: Closed (submitted)