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.

usr/src/cmd/mdb/common/mdb/mdb_string.h (Diff revision 3)

Show all issues

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. 

   Yuri Pankov March 26, 2017, 6:26 a.m.

   Not sure what to do here (about open issue), so I'll just mark it as dropped.

usr/src/head/fts.h (Diff revision 3)

Show all issues

Please use the correct illumos header guard, no trailing '_' character.

usr/src/head/fts.h (Diff revision 3)

Show all issues

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.

usr/src/lib/libc/amd64/Makefile (Diff revision 3)

Show all issues

Why create a new directory? Can't we just put them in port/gen?

1. 

   Yuri Pankov March 26, 2017, 6:26 a.m.

   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.

usr/src/man/man3c/fts.3c (Diff revision 3)

Show all issues

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. 

   Yuri Pankov March 26, 2017, 6:26 a.m.

   Will do.

2. 

   Robert Mustacchi March 27, 2017, 5:41 a.m.

   Looks like this was missed in the most recent update?  Would it help if I provided some text?

3. 

   Yuri Pankov March 27, 2017, 5:42 a.m.

   Yes, I did everything except this item, and it would really help, thanks Robert!

4. 

   Robert Mustacchi March 27, 2017, 7:18 p.m.

   How's something like this:
   
   737a738,767
   > .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 .
   786c816,817
   < .Sy Safe .
   ---
   > See
   > .Sx Locking .
   791c822,823
   < .Xr qsort 3C
   ---
   > .Xr qsort 3C ,
   > .Xr attributes 5
   

5. 

   Yuri Pankov March 28, 2017, 4:48 a.m.

   Thanks, added!

usr/src/man/man3c/malloc.3c (Diff revision 3)

Show all issues

Missing reallocarray up here?

usr/src/man/man3c/malloc.3c (Diff revision 3)

Show all issues

Or reallocarray?

Thanks for the follow ups here, Yuri. I'm happy being a reviewer on this.