Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Re: reallocarr(3) cleanup

3 views
Skip to first unread message

Kamil Rytarowski

unread,
Jul 11, 2015, 9:23:27 AM7/11/15
to
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 17.03.2015 23:56, Kamil Rytarowski wrote:
> Hello,
>
> I'm attaching a patch with reallocarr{.ay}(3) cleanup: - merge
> reallocarr.3 to malloc.3 for consistency, - make the reallocarr(3)
> documentation more detailed, - write history in malloc.3 of
> function allocations, - add in malloc.3 warning about possible
> overflows, - add erallocarr(3) to libutil, - add missing Id tag to
> reallocarray.c, - sync reallocarr prototype with documentation (num
> -> number), - note that reallocarr first appeared in .Nx 8 not 7
> (am I right about it?).
>
> Please review the English correctness.
>
> src/lib/libc/stdlib/reallocarray.3 is without licensing notes!
> please add it.
>
> CC: authors of the new function.
>
> Thank you for your time!
>

I noticed that I forgot to attach the patch..
https://github.com/krytarowski/netbsd/commit/75c12bd4e1458485e585fab3980
f250fdb80329b

I'm planning to commit on Monday (13 Jul) from it the following changes:
- - add erallocarr(3) to libutil,
- - add missing Id tag to reallocarray.c,
- - sync reallocarr prototype with documentation (num -> number),
- - reorder memcpy(3) and save errno -- for safety as memcpy(3) might
change it.

I will leave man pages for now.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVoRehAAoJEEuzCOmwLnZsP7wP/3Gzjyiq3Fyu6ltQCnRKYKXT
upmpcH3hYdLXwRisU/xsxMtz1vsx6EkFDWb09WDxY6k/S2lFTf+ulOcTiTWEWcyY
uIKpy0wWigr2sYhhMB9vcEMxq+oVuFyxMk62Gbb26rqhRm9T8J43w/BlzIHTJM9I
q9TcKbUXTc4nvz5ooaSDw/3l21x4n+K3hxrQMMzXApdJ7cVvE+ZHbgigmktVWNzN
oWcf1ixvLH3eiq+3rIGsY9sNsd1Sh1ujkFkq1YcsUg0lEn56aX1HmR3t1xwGbse+
V7iWcotqq06/zgh1srjQViaAfBWtHwdGGPrcpRxM4at+X4kv9+Te/JAHhfztv4Lv
0t6z/aWXup7j6wJ2kTKjCq9j2IV9xZ9skcjqon6D/FByj17jsWiqSn8ZjFZ7gdql
MCDLidgEuMFy6jTCBG4qwodbmnQpAXVMaXMBWbgLo+g3aIKpQ3P3A88xspAxiJlS
Ux4rsUQEQIwaafSZNpRwjvYb+FavNoAyRQraFuH0DiDdh7d6lpDaBkCVHrO+nVMF
ikfk2IM4nQpqcrUMOdND8rW7x3qS3egmc/lxQEKcXv+jVtE4OSJNg2m/q3oqtTpB
0sqjlRTXcsG4P2TXVCnLb91CSIhokQiUINk2BzCJEBqP2GMxslRUJBsUImLJ9duH
gi3pl+264AsZPNxM3aZA
=q+l8
-----END PGP SIGNATURE-----

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-...@muc.de

Taylor R Campbell

unread,
Jul 11, 2015, 10:26:08 AM7/11/15
to
Date: Sat, 11 Jul 2015 15:18:31 +0200
From: Kamil Rytarowski <n...@gmx.com>

I noticed that I forgot to attach the patch..
https://github.com/krytarowski/netbsd/commit/75c12bd4e1458485e585fab3980
f250fdb80329b

Prefer attaching the patch to your message so the mailing list archive
keeps a copy of it for future reference.

I'm planning to commit on Monday (13 Jul) from it the following changes:

Separate commits, please!

- - add erallocarr(3) to libutil,

OK.

- - add missing Id tag to reallocarray.c,

Write `$NetBSD$'; `$NetBSD: $' doesn't work.

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

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.

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).

+.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.

Kamil Rytarowski

unread,
Jul 11, 2015, 10:34:25 AM7/11/15
to
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 11.07.2015 16:25, Taylor R Campbell wrote:
> Date: Sat, 11 Jul 2015 15:18:31 +0200 From: Kamil Rytarowski
> <n...@gmx.com>
>
> I noticed that I forgot to attach the patch..
> https://github.com/krytarowski/netbsd/commit/75c12bd4e1458485e585fab39
80
>
>
f250fdb80329b
>
> Prefer attaching the patch to your message so the mailing list
> archive keeps a copy of it for future reference.
>
> I'm planning to commit on Monday (13 Jul) from it the following
> changes:
>
> Separate commits, please!
>

This is the way I will do it.

> - - add erallocarr(3) to libutil,
>
> OK.
>
> - - add missing Id tag to reallocarray.c,
>
> Write `$NetBSD$'; `$NetBSD: $' doesn't work.
>
> - - sync reallocarr prototype with documentation (num -> number),
>
> 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.
>

Thanks!

> I saw some other changes in the commit. Please mention these in
> your email describing it!
>

Sorry, I will cherry-pick only the mentioned parts from the original
(old) patch. The rest (man-pages) won't be applied. I should be more
clear on skipping the rest.

I would like to keep the history of NetBSD functions anyway. But
please leave it (along with man-page changes) for later.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVoShIAAoJEEuzCOmwLnZs3PwQAK0US0pHyv52BoONO8iihX+U
9mmbGhaWjFWr5zb4TAFBqCoE3HeUfjMz7a9Zb905/cWtog5OAvTSyfr3BVuty8aY
Av56g61X/XlW0IwaIemA3XTStas9LdbIPC1ZfUDr7U9qcD7Zmy2+vVCP262DNlpE
U84G2zW+JziXnnuaGBQo0GMyTGeIm/V3BPpXrfTw1RhVk+4PwwoIvtENu5Y6KFyR
9oyEXZz+LZjWuTJJfitWn5VGVRtdo4aLP/xwvhc/weov32N6P5GoqmJmjxRJJb3m
EUplNg0pdHPTRCIvi5JGSZA6kNMh9NJECKICQr+sQFKR0hzS+IhLcfe5aGT9ICKd
W5tcRRy9QgvAOaMdfH89aFcbsuN++/OVJoanP2HNGP3+UI8Yt79CsNq8e1Kibz9P
DDZ8Hhe8geDO28tsIMpM+PcGLq+jmwyuQvNVanCVzSzDv7EJYJhUjBpURW+UtzQp
2HyJ7RQbk2tcGHL6Ko4WfbZoofDX1BmK/GsJCxOWBTuyHXZcAEFjIzScBdPN3GOO
n8dx1m5lzF4RAauZW1/mzFXlSsLpJfv9VqdD8ITLg/oqtB3iPEPSTU8FcfnlSYYz
bEbJpXZweHljS8SLbaVhep4cvvajKeysZu2MNXvC+AsJiB8Q8ovKTgP+b2vcL1dq
VKFn0WXcWzqoH0JKbLfw
=TNWb
-----END PGP SIGNATURE-----

Christos Zoulas

unread,
Jul 11, 2015, 4:13:57 PM7/11/15
to
On Jul 11, 2:25pm, campbell+netbsd...@mumble.net (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 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

Kamil Rytarowski

unread,
Jul 26, 2015, 1:14:36 PM7/26/15
to
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 11.07.2015 22:13, Christos Zoulas wrote:
> On Jul 11, 2:25pm, campbell+netbsd...@mumble.net
> (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.

Everything from this patch-set has been committed.

Thank you!

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVtRRIAAoJEEuzCOmwLnZsrOUQAIsg7rpwk24goM24aIOEr1d4
L7RJp0GoaPNduiutjLCf726zvyK7fM015je/5njTtVEPkBtFcBj/hlYlZE4TIKcC
nWNMFgufkr4DGjZTWqUFTO0rOPx2WNsNCzLXxBHjoPXSg97dcvyx76BzgMFmOBRI
tJbLsqqeqDfjTYhIYhCknxLrtLfzktWPUHwmf7hyHYr814nHIMcqRsvAsROgGSM3
X9+lTWhA/qIyqOD1zwdCvAqfA5i4N5Uf6HdTRzfAxjyKuB+I8GUG9mfhMT2XZ3BU
aU2HhEZ/7c3iwmy5S6XFxVnXesG6Cg/yL6iTPYDzBC1ijZeCGyNpBzhWlwUOcCj4
lxXAFn/yBpqwx3Q+aovFcJmhj0csR3tJ/909fejtAQE5uzaF88xaM4OpOMVg6Rr3
KcdO69z2HWfzP44W/S1hHVUYMTvQNsJXkjqDkNOEU/TbJJe4T5SManaTMBzwv7Ve
xrVX51hx/ZJXDZnP8qRGNwwJPzQqgJ9kV87rcUo7I86rk80BQwNp3AlN9UQqFz9S
/YARlJKhQaU5ubPMpQcChT2C7djRVvNzj3b958AehN0yfZ9ceJ3FgIv80Uv+A+dV
k7tWFleOQ/bYbE+eF61t6fjXmKnq5ouEB1U/mbfycsRVwI0T7dVerAEWG8TZfoky
bzcvWJFwA0QZJ8RAhP/I
=fnbx
-----END PGP SIGNATURE-----
0 new messages