Re: reallocarr(3) cleanup

[christos%zoulas.com@localhost (Christos Zoulas)]

On Jul 11,  2:25pm, campbell+netbsd-tech-userlevel%mumble.net@localhost (Taylor R Campbell) wrote:
-- Subject: Re: reallocarr(3) cleanup

|    I'm planning to commit on Monday (13 Jul) from it the following changes:
| 
| Separate commits, please!
| 
|    - - add erallocarr(3) to libutil,
| 
| OK.

OK.

| 
|    - - add missing Id tag to reallocarray.c,
| 
| Write `$NetBSD$'; `$NetBSD: $' doesn't work.

both work...

|    - - sync reallocarr prototype with documentation (num -> number),
| 
| OK.

OK.

| 
|    - - reorder memcpy(3) and save errno -- for safety as memcpy(3) might
|    change it.
| 
| OK, although I don't think there is any danger that anyone's memcpy
| might clobber errno.

If memcpy changes errno, then you are in a world of hurt...

| I saw some other changes in the commit.  Please mention these in your
| email describing it!
| 
|    -	radixsort.3 rand48.3 rand.3 random.3 reallocarr.3 reallocarray.3 \
|    +	radixsort.3 rand48.3 rand.3 random.3 reallocarray.3 \ 
| 
|    -MLINKS+=malloc.3 calloc.3 malloc.3 realloc.3 malloc.3 free.3
|    +MLINKS+=malloc.3 calloc.3 malloc.3 realloc.3 malloc.3 reallocarr.3
|    +MLINKS+=malloc.3 free.3
| 
| As discussed before, please don't move reallocarr into the malloc(3)
| man page.  It would be worthwhile to add an xref in the malloc(3) man
| page, and perhaps even an illustrative example of how malloc, calloc,
| and realloc are not satisfactory for arrays with an xref to
| reallocarr(3), but the man pages should be separate.
| 
|    +.Sh HISTORY
|    +A
|    +.Fn free
|    +internal kernel function and a predecessor to
|    +.Fn malloc ,
|    +.Fn alloc ,
|    +first appeared in
|    +.At v1 .
|    +C library functions
|    +.Fn alloc
|    +and
|    +.Fn free
|    +appeared in
|    +.At v6 . 
|    ...

| The malloc(3) man page isn't really the place for the history of
| allocators in C.  We already have a brief note about the history of
| the current implementation in jemalloc(3).
 
I think that's ok (mentioning history), but not moving reallocar*
into the man pages from malloc is not. Historical perspective is
always welcome for me.

|    +.Sh CAVEATS
|    +When using
|    +.Fn malloc
|    +or
|    +.Fn realloc
|    +be wary of overflow when there is multiplication in the
|    +.Fa size
|    +argument. 
| 
| We already have text in the man page about integer overflow -- no need
| to duplicate the message.  The longer the man page is, the less likely
| I am to read through the whole thing.  Signed integer overflow is a
| generic issue not related to malloc; wrapping integer overflow is
| already addressed.
| 
| If you don't like the way it's addressed, that's another thing.  But
| in this case, I think it is better to be associated with illustrative
| examples than to be an abstract paragraph of text.

I agree, mentioning integer overflow multiple times is not productive,
but clarifying it the the place it is mentioned should be fine.

christos