Re: svn commit: r334275 - head/lib/libc/string
Hello Marcelo,
Thanks for working on that :)
>
> tofree = string = strdup("abc,def,ghi");
> -assert(string != NULL);
> +if (string != NULL)
> + while ((token = strsep(, ",")) != NULL)
> + printf("%s\en", token);
Please notice:
```
If *stringp is initially NULL, strsep() returns NULL.
```
So I even not sure if you need to check strdup() at this point.
>
> -while ((token = strsep(, ",")) != NULL)
> - printf("%s\en", token);
> -
> free(tofree);
> +free(string);
Here you introduced potential double free.
At the end of loop the 'string' will be equals to NULL so there is no point to
free it. If somebody would use this code as example and he from any other reason
would stop at any other point the string will be pointing to the middle of
'tofree' variable which you already freed.
Thanks,
--
Mariusz Zaborski
oshogbo//vx | http://oshogbo.vexillium.org
FreeBSD commiter| https://freebsd.org
Software developer | http://wheelsystems.com
If it's not broken, let's fix it till it is!!1
signature.asc
Description: PGP signature
Re: svn commit: r334275 - head/lib/libc/string
In message <20180528155019.w2...@besplex.bde.org>, Bruce Evans writes: >Note that assert() can't be used in signal handlers since it is required >to print to stderr due to C not having any output methods except stdio. That is precisly why I added abort2(2) more than 10 years ago. -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 p...@freebsd.org | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r334275 - head/lib/libc/string
On Mon, 28 May 2018, Marcelo Araujo wrote:
Log:
Update strsep(3) EXAMPLE section regards the usage of assert(3).
As many people has pointed out, using assert(3) shall be not the best approach
to verify if strdup(3) has allocated memory to string.
Reviewed by: imp
Modified: head/lib/libc/string/strsep.3
==
--- head/lib/libc/string/strsep.3 Mon May 28 04:38:10 2018
(r334274)
+++ head/lib/libc/string/strsep.3 Mon May 28 05:01:42 2018
(r334275)
@@ -31,7 +31,7 @@
.\"@(#)strsep.38.1 (Berkeley) 6/9/93
.\" $FreeBSD$
.\"
-.Dd December 5, 2008
+.Dd May 28, 2018
.Dt STRSEP 3
.Os
.Sh NAME
@@ -86,12 +86,12 @@ to parse a string, and prints each token in separate l
char *token, *string, *tofree;
tofree = string = strdup("abc,def,ghi");
-assert(string != NULL);
+if (string != NULL)
+ while ((token = strsep(, ",")) != NULL)
+ printf("%s\en", token);
-while ((token = strsep(, ",")) != NULL)
- printf("%s\en", token);
-
free(tofree);
+free(string);
.Ed
.Pp
The following uses
This is even worse than before. Errors are now mishandled in all cases
by silently ignoring them.
On most arches, null pointers are handled better than with assert() by
the default error handling of sending a SIGSEGV to the process; this normally
terminates it and dumps core.
All assert() does is give a message that is only slightly better than nothing
when it work, and then sends a SIGABRT to the process after breaking
restartability by doing this in the non-returning functions abort(). That
is without NDEBUG. NDEBUG gives the better default error handling.
Note that assert() can't be used in signal handlers since it is required
to print to stderr due to C not having any output methods except stdio.
Even printing to stderr is fragile, especially after malloc() failures,
since stderr might be buffered and delayed allocation of its buffer might
fail. The FreeBSD implementation falls back to unbuffered then (or fails
in setvbuf()), but the C standard doesn't require this.
malloc() failures "can't happen" anyway, especially for strdup(). I
used to test them using rlimits. IIRC, ulimit -d and ulimit -m used
to work to restrict malloc() when malloc() used sbrk(). But now
malloc() uses mmap() and limits don't work for it. Also, malloc()
on at least amd64 now takes more than 2MB before doing anything (2MB
for __je_extents_rtree and a mere few hundred KB for other parts of
malloc()). malloc() might fail, but hundreds if not thousands of
small strdup()s can probably be done in just the slop required to
fit malloc()'s overhead.
Bruce
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r334275 - head/lib/libc/string
Author: araujo
Date: Mon May 28 05:01:42 2018
New Revision: 334275
URL: https://svnweb.freebsd.org/changeset/base/334275
Log:
Update strsep(3) EXAMPLE section regards the usage of assert(3).
As many people has pointed out, using assert(3) shall be not the best approach
to verify if strdup(3) has allocated memory to string.
Reviewed by: imp
MFC after:4 weeks.
Sponsored by: iXsystems Inc.
Differential Revision:https://reviews.freebsd.org/D15594
Modified:
head/lib/libc/string/strsep.3
Modified: head/lib/libc/string/strsep.3
==
--- head/lib/libc/string/strsep.3 Mon May 28 04:38:10 2018
(r334274)
+++ head/lib/libc/string/strsep.3 Mon May 28 05:01:42 2018
(r334275)
@@ -31,7 +31,7 @@
.\"@(#)strsep.38.1 (Berkeley) 6/9/93
.\" $FreeBSD$
.\"
-.Dd December 5, 2008
+.Dd May 28, 2018
.Dt STRSEP 3
.Os
.Sh NAME
@@ -86,12 +86,12 @@ to parse a string, and prints each token in separate l
char *token, *string, *tofree;
tofree = string = strdup("abc,def,ghi");
-assert(string != NULL);
+if (string != NULL)
+ while ((token = strsep(, ",")) != NULL)
+ printf("%s\en", token);
-while ((token = strsep(, ",")) != NULL)
- printf("%s\en", token);
-
free(tofree);
+free(string);
.Ed
.Pp
The following uses
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
