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

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

Information
yuripv
illumos-gate
master
5428
5fda943...
Reviewers
general 
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
yuripv
yuripv
rm
rm 
  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) 
     
     
    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. 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) 
     
     
    Show all issues 
    Please use the correct illumos header guard, no trailing '_' character.
  4. 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.
  5. 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. 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) 
     
     
    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. 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:
      

      

      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. Thanks, added!
  7. usr/src/man/man3c/malloc.3c (Diff revision 3) 
     
     
    Show all issues 
    Missing reallocarray up here?
  8. usr/src/man/man3c/malloc.3c (Diff revision 3) 
     
     
    Show all issues 
    Or reallocarray?
  9. 
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    10. 




  

 
















 



 

  

   




yuripv




   

   

  

 



 















 



 

  

   




yuripv




   

   

  

 



 















 



 

  

   




yuripv




   

   

  

 



 















 



 

  

   




yuripv




   

   

  

 



 















 



 

  

   




yuripv




   

   

  

 



 















 



 

  

   




yuripv




   

   

  

 



 















 





 

  

   




rm




   

   







  

 



 

  


   

   

    
 
rm


    
   


  

  

  



    




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

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




yuripv




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 







 


 










  

 









     

    

   

  

  

   
   Loading...