realloc

[Roy]

Hi all :
My code below :

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL)
tmp = realloc(s,strlen(t) + 1);
else
tmp = realloc(s,strlen(s) + strlen(t) + 1);
if (tmp == NULL) {
fputs("realloc error",stderr);
exit(1);
}
while (*tmp++); // search for '\0' and stop
tmp--; //the position of '\0'
while (*tmp++ = *t++) ;
s = tmp;
return s;
}

int main()
{
char *tmp;
char *s;

s = NULL;
tmp = cat(s,"oh");
printf("%s\n",tmp);
tmp = cat(s,"haha");
printf("%s\n",tmp);
return 0;
}
my problem is when I run the program the reslut is some blanks. I just
wrote a small routine like strcat and check the realloc and tmp pointer
carefully but found nothing.

 

[baumann@pan]

Hi all :
My code below :

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL)
tmp = realloc(s,strlen(t) + 1);
else
tmp = realloc(s,strlen(s) + strlen(t) + 1);
if (tmp == NULL) {
fputs("realloc error",stderr);
exit(1);
}
while (*tmp++); // search for '\0' and stop

the above statement is not right, when s = null, what do you expect the
*tmp is?

 

[Roy]

correct here:
if (s == NULL) {
tmp = realloc(s,strlen(t) + 1);
while(*tmp++ = *t++); // copy *t to *tmp.
} else { the above statement is not right, when s = null, what do you expect
the
*tmp is?
Now handled the s=null but still blanks...

 

[pete]

Hi all :
My code below :

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL)
tmp = realloc(s,strlen(t) + 1);
else
tmp = realloc(s,strlen(s) + strlen(t) + 1);
if (tmp == NULL) {
fputs("realloc error",stderr);
exit(1);
}
while (*tmp++); // search for '\0' and stop
tmp--; //the position of '\0'
while (*tmp++ = *t++) ;
s = tmp;
return s;
}

int main()
{
char *tmp;
char *s;

s = NULL;
tmp = cat(s,"oh");
printf("%s\n",tmp);
tmp = cat(s,"haha");
printf("%s\n",tmp);
return 0;
}

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL) {
tmp = realloc(s, strlen(t) + 1);
*tmp = '\0';
} else {
tmp = realloc(s, strlen(s) + strlen(t) + 1);
}
if (tmp == NULL) {
fputs("realloc error",stderr);
exit(EXIT_FAILURE);
}
s = tmp;
while (*tmp++ != '\0') {
;
}
tmp--;
while ((*tmp++ = *t++) != '\0') {
;
}
return s;
}

 

[Al Bowers]

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL) {
tmp = realloc(s, strlen(t) + 1);
*tmp = '\0';

Here is a slight flaw. You do not want to assign to *tmp
BEFORE you have checked tmp value for a possible NULL.

} else {
tmp = realloc(s, strlen(s) + strlen(t) + 1);
}
if (tmp == NULL) {
fputs("realloc error",stderr);
exit(EXIT_FAILURE);
}
s = tmp;
while (*tmp++ != '\0') {
;
}
tmp--;
while ((*tmp++ = *t++) != '\0') {
;
}
return s;
}

This definition will abort the program if dynamic allocation
fails. Another less virulent act on the executable would be
give a signal to the caller, via the return value, or some
other means, that the function was not successful in the
operation.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int cat(char **s1, const char *s2);

int main(void)
{
char *s = NULL;

if(cat(&s,"Hello"))
{
if(cat(&s," and Goodbye"))
printf("s = \"%s\"\n",s);
else puts("Memory allocation failure");
}
else puts("Memory allocation failure");
free(s);
return 0;
}

int cat(char **s1, const char *s2)
{
int ret = 0; /* value would indicate failure */

if(s1 && s2)
{
char *tmp;
size_t sz = *s1?strlen(*s1)+strlen(s2)+1:strlen(s2)+1;
if((tmp = realloc(*s1,sz)) != NULL)
{
if(!*s1) *tmp = '\0';
strcat(tmp,s2);
*s1 = tmp;
ret = 1; /* value would indicate success */
}
}
return ret;
}

 

[Flash Gordon]

Hi all :
My code below :

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL)
tmp = realloc(s,strlen(t) + 1);

At this point, the memory pointed to by tmp will be in a random state.
Also you might as well use malloc since you know s is NULL.

else
tmp = realloc(s,strlen(s) + strlen(t) + 1);
if (tmp == NULL) {
fputs("realloc error",stderr);
exit(1);
}
while (*tmp++); // search for '\0' and stop

So the while loop above could do anything if s is NULL, as it is on your
first call.

Also, you have just had the string searched for a '\0' by calling
strlen, so scanning it again seems a bit silly to me.

Also, please don't use // comments on Usenet. They do not survive line
wrapping and, for your information, were not standard in C89 although
they are a common extension. They are standard in C99, but most
implementations that I am aware of are not conforming C99 implementations.

tmp--; //the position of '\0'
while (*tmp++ = *t++) ;

Again, you know the length and could use memcpy which *may* be more
efficient.

s = tmp;
return s;
}

int main()
{
char *tmp;
char *s;

s = NULL;
tmp = cat(s,"oh");
printf("%s\n",tmp);
tmp = cat(s,"haha");

Here you are using s, but s will still be NULL, and loosing the pointer
you obtained by the previous call to cat.

printf("%s\n",tmp);
return 0;
}
my problem is when I run the program the reslut is some blanks. I just
wrote a small routine like strcat and check the realloc and tmp pointer
carefully but found nothing.

Try the following modified version...

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *cat(char *s, const char *t)
{
char *tmp;
size_t slen;
size_t tlen = strlen(t);

if (s == NULL)
slen = 0;
else
slen = strlen(s);
tmp = realloc(s,slen + tlen + 1);

if (tmp == NULL) {
free(s); /* improve chances of the fputs not running out of memory */
fputs("realloc error",stderr);
exit(1);
}
memcpy(tmp+slen,t,tlen);
tmp[slen+tlen]='\0';
return tmp;
}

int main(void)
{
char *tmp;
char *s = NULL;
tmp = cat(s,"oh");
printf("%s\n",tmp);
s = cat(tmp,"haha");
printf("%s\n",s);
return 0;
}

 

[pete]

Probably would be a better demonstration with
tmp = cat(tmp, "haha");

Here is a slight flaw. You do not want to assign to *tmp
BEFORE you have checked tmp value for a possible NULL.

Thank you.

This definition will abort the program if dynamic allocation
fails. Another less virulent act on the executable would be
give a signal to the caller, via the return value, or some
other means, that the function was not successful in the
operation.

/* I'm taking another shot. */

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *cat(char *s, const char *t);

int main(void)
{
char *s;

s = cat(NULL, "oh");
if (s != NULL) {
printf("%s\n", s);
}
s = cat(s, "haha");
if (s != NULL) {
printf("%s\n", s);
}
return 0;
}

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL) {
tmp = calloc(1, strlen(t) + 1);
} else {
tmp = realloc(s, strlen(s) + strlen(t) + 1);
}
s = tmp;
if (tmp != NULL) {
while (*tmp++ != '\0') {
;
}
tmp--;
while ((*tmp++ = *t++) != '\0') {
;
}
}
return s;
}

 

[pete]

/* I'm taking another shot. */

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *cat(char *s, const char *t);

int main(void)
{
char *s;

s = cat(NULL, "oh");
if (s != NULL) {
printf("%s\n", s);
}
s = cat(s, "haha");
if (s != NULL) {
printf("%s\n", s);
}
return 0;
}

/*
** I got it into my head that writing cat using strcat,
** might be cheating in some way, but ...
*/

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL) {
tmp = calloc(1, strlen(t) + 1);
} else {
tmp = realloc(s, strlen(s) + strlen(t) + 1);
}
s = tmp;
if (s != NULL) {
strcat(s, t);
}
return s;
}

 

[Michael Knaup]

/*
** I got it into my head that writing cat using strcat,
** might be cheating in some way, but ...
*/

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL) {
tmp = calloc(1, strlen(t) + 1);
} else {
tmp = realloc(s, strlen(s) + strlen(t) + 1);
}
s = tmp;

Why reusing s here, s is only a copy of your s in the main function. So if
you want to change your "main" s you have to use a pointer to a pointer
like All did.

if (s != NULL) {
strcat(s, t);

Maybe the use of malloc/realoc and memcpy is a "little" bit faster than your
solution

}
return s;
}

This one should work

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#if __STDC_VERSION__ < 199901L
# define restrict
#endif

char* concat (char **s, restrict const char *t)
{
char *tmp = NULL;

if (NULL == t) {
if (NULL != s) {
tmp = *s;
}
} else {
size_t length = 1u + strlen(t);

if (NULL == s || NULL == *s) {
tmp = malloc (length);
if (NULL != tmp) {
memcpy(tmp, t, length);
if (NULL != s) {
*s = tmp;
}
}
} else {
size_t old_len = strlen(*s);
tmp = realloc (*s, old_len + length);
if (NULL != tmp) {
memcpy (tmp + old_len, t, length);
*s = tmp;
}
}
}
return tmp;
}

int main(void)
{
char *string = NULL;
char *tmp;

concat(&string, "oh");
printf ("*%p == %s\n", string, string);
tmp = string;
do {
concat(&string, "haha");
} while (tmp == string);
printf ("*%p (%zd) == %s\n", string, strlen(string), string);

free (string);

return EXIT_SUCCESS;
}

 

[Michael Knaup]

And true you should check against NULL after calling concat

int main(void)
{
char *string = NULL;
char *tmp;

concat(&string, "oh");

if (NULL == string)
return EXIT_FAILURE;

printf ("*%p == %s\n", string, string);
tmp = string;
do {

if (NULL == concat(&string, "haha"))
break;

 

[pete]

Why reusing s here,
s is only a copy of your s in the main function. So if
you want to change your "main"
s you have to use a pointer to a pointer
like All did.

I changed the value of s in main this way:
s = cat(NULL, "oh");
s = cat(s, "haha");
I wanted to return either a pointer to memory,
or return NULL, from cat.

free (string);

I like that line of code.

My personal preference is to call free from the same
function that makes the call to *alloc.,
except for lists.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

int main(void)
{
char *s;
char *old_s;

old_s = s = malloc(sizeof "oh");
if (s != NULL) {
puts(strcpy(s, "oh"));
}
if (s != NULL) {
s = realloc(s, strlen("oh") + sizeof "haha");
} else {
s = calloc(1, sizeof "haha");
}
if (s != NULL) {
puts(strcat(s, "haha"));
} else {
s = old_s;
}
free(s);
return 0;
}

 

[Michael Knaup]

I changed the value of s in main this way:
s = cat(NULL, "oh");
s = cat(s, "haha");

But what happens when the memory allocation in cat fails?
I think we have a memory leckage then, don't we?

I wanted to return either a pointer to memory,
or return NULL, from cat.


I like that line of code.

My personal preference is to call free from the same
function that makes the call to *alloc.,
except for lists.

Yes thats always a good idea, and you are free to do so with both solutions.
And yes you can do the whole task in place, but do you wanna mantain this
code?

 

[pete]

But what happens when the memory allocation in cat fails?
I think we have a memory leckage then, don't we?

Yes we do.
I should have used an old_s variable there too.
Especially since the subject is realloc,
and saving the value of the old pointer is very much
a part of using realloc correctly.

Yes thats always a good idea,
and you are free to do so with both solutions.
And yes you can do the whole task in place,
but do you wanna mantain this code?

I don't know.
I can't really see this code
being anything other than a realloc exercise.

 

[pete]

/* improve chances of the fputs not running out of memory */

#include <stdio.h>

int fput_s(const char *s, FILE *stream);

int main(void)
{
fput_s(
"What kind of memory usage "
"are you envisioning for fputs?\n",
stdout
);
return 0;
}

int fput_s(const char *s, FILE *stream)
{
while (*s != '\0') {
if (putc(*s, stream) == EOF) {
return EOF;
}
++s;
}
return 0;
}

 

[Al Bowers]

Michael Knaup wrote:

Maybe the use of malloc/realoc and memcpy is a "little" bit faster than your
solution

The use of memcpy is certainly a good option. The use of the
malloc/realloc combination is not neccessary. Just use
function realloc to do the allocations(see below).
I believe that was the intent of the OP.

Function cat below, is an attempt rewrite of this function
concat. I believe all the operations are safe.

char* concat (char **s, restrict const char *t)
{
char *tmp = NULL;

if (NULL == t) {
if (NULL != s) {
tmp = *s;
}
} else {
size_t length = 1u + strlen(t);

if (NULL == s || NULL == *s) {
tmp = malloc (length);
if (NULL != tmp) {
memcpy(tmp, t, length);
if (NULL != s) {
*s = tmp;
}
}
} else {
size_t old_len = strlen(*s);
tmp = realloc (*s, old_len + length);
if (NULL != tmp) {
memcpy (tmp + old_len, t, length);
*s = tmp;
}
}
}
return tmp;
}

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *cat(char **s1, const char *s2);

int main(void)
{
char *s = NULL;

if(cat(&s,"Hello"))
{
if(cat(&s," and Goodbye"))
printf("s = \"%s\"\n",s);
else puts("Memory allocation failure");
}
else puts("Memory allocation failure");
free(s);
s = cat(NULL,"This string generated with NULL 1st argument");
if(s) printf("s = \"%s\"\n",s);
free(s);
return 0;
}

char *cat(char **s1, const char *s2)
{
char *ret = NULL;

if(s2)
{
char *tmp;
size_t sz1 = (!s1||!*s1)?0:strlen(*s1);
size_t sz2 = strlen(s2);
if((tmp = realloc(s1?*s1:NULL,sz1+sz2+1)) != NULL)
{
memcpy(tmp+sz1,s2,sz2+1);
ret = (s1)?*s1 = tmp:tmp;
}
}
return ret;
}

 

[Flash Gordon]

Flash Gordon wrote:




#include <stdio.h>

int fput_s(const char *s, FILE *stream);

int main(void)
{
fput_s(
"What kind of memory usage "
"are you envisioning for fputs?\n",
stdout
);
return 0;
}

I don't care, but it might either allocate or increase the size of a
buffer. I've recently been working with a DB wrapper layer that does
clever things which can lead to you running out of resources trying to
close a table because you have run out of resources, unless you first
close all temporary tables. So strange things can happen on running out
of resources, and that free was a cheap and easy way to reduce the
chances of something strange happening.

It was also a subtle hint to the OP that it was still allocated in case
later s/he wants to do something more intelligent on on a realloc failure.

Also, I'm happy if that is the worst thing anyone here comments on with
a piece of my code.

 

[CBFalconer]

Hi all :
My code below :

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL)
tmp = realloc(s,strlen(t) + 1);
else
tmp = realloc(s,strlen(s) + strlen(t) + 1);
if (tmp == NULL) {
fputs("realloc error",stderr);
exit(1);
}
while (*tmp++); // search for '\0' and stop
tmp--; //the position of '\0'
while (*tmp++ = *t++) ;
s = tmp;
return s;
}

int main()
{
char *tmp;
char *s;

s = NULL;
tmp = cat(s,"oh");
printf("%s\n",tmp);
tmp = cat(s,"haha");
printf("%s\n",tmp);
return 0;
}
my problem is when I run the program the reslut is some blanks. I just
wrote a small routine like strcat and check the realloc and tmp pointer
carefully but found nothing.

Here is a revision that appears to work. Notice the differences,
including the use of blanks in the statements.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char *cat(char *s, const char *t)
{
char *tmp;

if (s == NULL) {
tmp = realloc(s, strlen(t) + 1);
if (tmp) *tmp = '\0';
}
else
tmp = realloc(s, strlen(s) + strlen(t) + 1);
if (tmp == NULL) {
fputs("realloc error", stderr);
exit(1);
}
else {
s = tmp;
while (*tmp++) continue; /* search for '\0' and stop */
tmp--; /* the position of '\0' */
while (*tmp++ = *t++) continue;
}
return s;
}

int main(void)
{
char *s;

s = NULL;
s = cat(s, "oh");
printf("%s\n", s);
s = cat(s, "haha");
printf("%s\n", s);
return 0;
}


--
Some informative links:
http://www.geocities.com/nnqweb/
http://www.catb.org/~esr/faqs/smart-questions.html
http://www.caliburn.nl/topposting.html
http://www.netmeister.org/news/learn2quote.html

 

[Michael Knaup]

char *cat(char **s1, const char *s2)
{
char *ret = NULL;

if(s2)
{
char *tmp;
size_t sz1 = (!s1||!*s1)?0:strlen(*s1);
size_t sz2 = strlen(s2);
if((tmp = realloc(s1?*s1:NULL,sz1+sz2+1)) != NULL)
{

I think we have undefined behavior in the case of "self-concatenation"
(cat(&s, s) so we should check for this case.

memcpy(tmp+sz1,s2,sz2+1);
ret = (s1)?*s1 = tmp:tmp;
}
}
return ret;
}

char* StrConcat (char ** pS0, const char * const s1)
{
char *pTmp = (pS0 ? *pS0 : (pS0 = &pTmp, NULL));

if (s1) {
register size_t l0 = (pTmp ? strlen (pTmp) : 0u);
register size_t l1 = 1u + strlen (s1);
if ((pTmp = realloc (pTmp, l0 + l1))) {
if (*pS0 == s1) {
memcpy (pTmp + l0, pTmp, l0);
pTmp[l0 << 1] = '\000';
} else {
memcpy (pTmp + l0, s1, l1);
}
*pS0 = pTmp;
}

}

return pTmp;
}
#define StrAlloc(s1) StrConcat(NULL, (s1))
#define StrFree(s1) (free(s1), (s1) = NULL)

 

[Al Bowers]

Al Bowers wrote:



I think we have undefined behavior in the case of "self-concatenation"
(cat(&s, s) so we should check for this case.

I assume you are referring to the overlap hazard involving function
memcpy. If one is to write the function cat to protect against the
overlapUB, you should do more than check for the case of
"self-concatenation". You should prevent all possible cases of
overlap UB, ie. (cat(&s,s+1) where strlen of s is greater than 1.
memmove(tmp+sz1,s2,sz2+1);

Standard C provides function memmove which is similiar to memcpy
but eliminates the overlap problem.

ret = (s1)?*s1 = tmp:tmp;
}
}
return ret;
}

char* StrConcat (char ** pS0, const char * const s1)
{
char *pTmp = (pS0 ? *pS0 : (pS0 = &pTmp, NULL));

if (s1) {
register size_t l0 = (pTmp ? strlen (pTmp) : 0u);
register size_t l1 = 1u + strlen (s1);
if ((pTmp = realloc (pTmp, l0 + l1))) {
if (*pS0 == s1) {
memcpy (pTmp + l0, pTmp, l0);
pTmp[l0 << 1] = '\000';
} else {
memcpy (pTmp + l0, s1, l1);
}

This if-else statement only catches the overlap UB then
*pS0 == s1. Cases of overlap UB where *ps0 != s1 will
make the StrCancat function subject to failure.

 

[Michael Knaup]

I assume you are referring to the overlap hazard involving function
memcpy. If one is to write the function cat to protect against the
overlapUB, you should do more than check for the case of
"self-concatenation". You should prevent all possible cases of
overlap UB, ie. (cat(&s,s+1) where strlen of s is greater than 1.

No, not really, the pointer to s2 may become invalid by the realloc call if
s2 == s1 and s1 != realloc(s1, new_size).

In C99 you also may solve the problem by declaring s2 as
const char * restrict s2 or
const char *const restrict
then self-concatenation is disallowed

memmove(tmp+sz1,s2,sz2+1);

Standard C provides function memmove which is similiar to memcpy
but eliminates the overlap problem.

As I wrote memmove will not solve the problem and no you have not to use
memmove in the case of partial "self-concatenation". Because the only byte
which would be a problem is the terminating '\000' of s1 and then it is
enougth to handle string termination separatley. But again s2 may be
invalid and you have to handle this.
ptrdiff_t d0 = s1 - *pS0;

if ((pTmp = realloc (pTmp, l0 + l1))) {
if (0 <= d0 && (size_t)d0 <= l0) {
--l1;
memcpy (pTmp + l0, pTmp + d0, l1);
pTmp[l0 + l1] = '\000';

I think, this will solve the failure for partial "self-concatenation"