reopen 422610
thanks
Mario,
It kinda defeats the purpose of using -Werror and having a compiler clever
enough to output warnings about misuse of realloc(), if you are going to
"fix" it in a way that works around the warning while still being a misuse
of realloc()!
for(lines=0,line=malloc(BUFSIZ);line &&
fgets(line,BUFSIZ,stdin);lines++,line=malloc(BUFSIZ)) {
size_t len = strlen(line);
+ char** tmpaddr=NULL;
msg_len+=len;
- realloc(line, len+1);
+ tmpaddr = realloc(line, len+1);
message=realloc(message,sizeof(void*) * lines+1);
message[lines]=line;
You are *not allowed* to reference the original value of 'line' after it's
been passed as the first argument to a successful realloc(), because the C
library is allowed to *move* the allocated memory to a different address as
part of the reallocation, invalidating all references to the old address.
The use of a tmpaddr variable is fine, because among other things it gives
you the means of checking for errors in realloc; but the actual error
checking is missing, and the remaining reference to 'line' after the realloc
is still invalid and a potential segfault. Even if it doesn't result in a
segfault, if glibc chooses to move the pointer because of a significant
reduction in block size, referencing 'line' may mean you're referencing
memory that will be overwritten by a subsequent call to malloc().
I'm reopening this bug at the same severity, since the fundamental reason
for the build failure has not been fixed.
FWIW, and given that the code seems concerned about efficient use of memory,
I would argue here for using a single, static char line[BUFSIZ] buffer and
strdup()ing the contents directly to message[lines]. This gives you a
constant overhead of 8k, but you effectively have that all the way through
this loop anyway and the static buffer will save you the runtime overhead of
repeated realloc()s.
E.g.:
char line[BUFSIZ];
for (lines=0;fgets(line,BUFSIZ,stdin);lines++) {
msg_len+=strlen(line);
message=realloc(message,sizeof(void*) * lines+1);
message[lines] = strdup(line);
if (!message[lines]) {
fprintf(stderr,_("%s: OOM:
%s\n"),argv[0],strerror(errno));
exit(DEFERAL);
}
}
Cheers,
--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
[EMAIL PROTECTED] http://www.debian.org/
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
