• Listen to a special audio message from Bill Roper to the Hive Workshop community (Bill is a former Vice President of Blizzard Entertainment, Producer, Designer, Musician, Voice Actor) 🔗Click here to hear his message!
  • Read Evilhog's interview with Gregory Alper, the original composer of the music for WarCraft: Orcs & Humans 🔗Click here to read the full interview.

dynamic strings c

Status
Not open for further replies.
Level 10
Joined
Sep 19, 2011
Messages
527
hi, i recently began to program in c (i'm very noob) and i have problems appending chars to dynamic allocated strings.

i created this function to append strings:

Code:
char *appstr(char *s, const char *d)
{
	size_t slen = 0;
	size_t dlen = 0;
	char *ns;

	if (s)
		slen = strlen(s);

	if (d)
		dlen = strlen(d);

	ns = malloc(sizeof(char) * (slen+dlen+1));
	strcpy(ns, "");

	if (s)
		strcat(ns, s);

	if (d)
		strcat(ns, d);

	free(s);

	return ns;
}

and it works pretty well, except in this piece of code:

Code:
do {
		ch = fgetc(dependency_file);

		if (ch == '\n' || ch == EOF) {
			dependency = Dependency_create_from_url(dependency_url);
			Dependency_set_version(dependency, dependency_version);

			printf("%s,%s\n", dependency_url, dependency_version);

			if (DependencyContainer_add(dc, dependency) == DEPENDENCY_CONTAINER_VERSION_ERROR) {
				if (display_errors == 1) {
					printf(" [ERROR] Dependency %s is required in different versions\n", dependency->name);
				}
			}

			free(dependency_url);
			free(dependency_version);

			dependency_url = NULL;
			dependency_version = NULL;

			in_version = 0;
		} else if (ch == ',') {
			in_version = 1;
		} else {
			if (in_version == 0) {
				dependency_url = appstr(dependency_url, &ch);
			} else {
				dependency_version = appstr(dependency_version, &ch);
			}
		}
	} while(ch != EOF);

which reads a file with a format url,version\n...

i'm testing it with this test-file:

Code:
[email protected]:Ruk33/WurstBuff.git,HEAD
[email protected]:Ruk33/WurstBuff.git,1234

and prints:

Code:
[email protected]:Ruk33/WurstBuff.git,HEAD
<corrupted/incorrect string>,<corrupted/incorrect string>

where <corrupted/incorrect string> is a bunch of weird characters.

as you can see, the first print is correct, but the second is wrong and i can't spot the problem out.

can anybody help me with this?

thanks, greetings.
 
Last edited:
Level 10
Joined
Sep 19, 2011
Messages
527
fixed:

Code:
	do {
		ch = fgetc(dependency_file);

		if (ch == '\n' || ch == EOF) {
			dependency_url = setstr(line);
			dependency_version = strrchr(line, ',');

			if (dependency_version) {
				// Remove version from line so we get only the url
				dependency_url[strlen(dependency_url) - strlen(dependency_version)] = '\0';

				// Remove ","
				dependency_version++;
			}

			dependency = Dependency_create_from_url(dependency_url);
			Dependency_set_version(dependency, dependency_version);

			if (DependencyContainer_add(dc, dependency) == DEPENDENCY_CONTAINER_VERSION_ERROR) {
				if (display_errors) {
					printf(" [ERROR] Dependency %s is required in different versions\n", dependency->name);
				}
			}
			
			free(line);
			free(dependency_url);
			free(dependency_version);

			line = NULL;
			dependency_url = NULL;
			dependency_version = NULL;
		} else {
			line = appstr(line, &ch);
		}
	} while(ch != EOF);
 
Last edited:

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,255
You should be using "realloc" instead of malloc to resize. This is because it might be able to resize without requiring a move which could save memory copies. Especially useful if you are appending a lot of small strings as memory allocation might be done in blocks of some granularity (a little buffer space between needing to move the entire string).

You should also be testing if the string pointer are not the NULL macro instead of if they are logically true. Although for most platforms both approaches work (since NULL is defined as 0 and logically true is defined as anything not 0) however this need not always be the case. It might seem to make the code more complicated (now using a comparison) however in reality the compiler (if it is good like gcc) will optimize that away.

Generally you want to avoid function calls which malloc memory and return it expecting the caller to free it when it is done as that couples implementation details to the caller (maybe the caller uses a different dynamic memory system?). Instead you should have functions which deal with the deallocation of memory returned by other complex functions, even if it deals with the deallocation of memory returned by many different functions. If it turns out the implementation is simply to free the argument the compiler should have no problem with in-lining it so in the end the performance is the same.
 
Level 10
Joined
Sep 19, 2011
Messages
527
thanks doctor :).


at the first i was using realloc, but it was throwing me some weird errors. i give it another try and i came up with this:

Code:
#include <stdlib.h>
#include <string.h>

#ifndef DSTR_H_INCLUDED
#define DSTR_H_INCLUDED

char *dstrcpy(char *source, const char *to_copy)
{
	if (source != NULL) {
		free(source);
	}

	if (to_copy != NULL) {
		source = malloc(sizeof(char) * (strlen(to_copy)+1));
		source = strcpy(source, to_copy);
	} else {
		source = malloc(sizeof(char));
		source = strcpy(source, "");
	}

	return source;
}

char *dstrcat(char *source, const char *append)
{
	if (source == NULL) {
		source = dstrcpy(source, "");
	}

	if (append != NULL) {
		source = realloc(source, sizeof(char) * (strlen(source)+strlen(append)+1));
		source = strcat(source, append);
	}

	return source;
}

char *dstrcatc(char *source, const char append)
{
	if (source == NULL) {
		source = dstrcpy(source, "");
	}

	if (append != '\0') {
		source = realloc(source, sizeof(char) * (strlen(source)+2));

		source[strlen(source)] = append;
		source[strlen(source)+1] = '\0';
	}

	return source;
}

#endif

tests:

Code:
#include "assert.h"
#include "../dstr.h"

void main()
{
	char *s = NULL;
	char *a = NULL;
	char ch = ',';

	s = dstrcpy(s, "something");
	ec("something", s);

	s = dstrcpy(s, "lorem");
	ec("lorem", s);

	a = dstrcat(a, NULL);
	ec("", a);

	a = dstrcat(a, "im");
	ec("im", a);

	a = dstrcat(a, " awesome");
	ec("im awesome", a);

	a = dstrcatc(a, ch);
	a = dstrcatc(a, ch);
	a = dstrcatc(a, '\n');
	ec("im awesome,,\n", a); // ---> FAILS a == "im awesome,,\n<weird character>

	free(s);
	free(a);
}

note: ec(e,g) -> equal_char(expected, given)

greetings and thanks again :).

edit: nvm about -2, if i alloc another char it fails.


edit: now it works perfectly with sprinf:

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

#ifndef DSTR_H_INCLUDED
#define DSTR_H_INCLUDED

char *dstrcpy(char *source, const char *to_copy)
{
	if (source != NULL) {
		free(source);
	}

	if (to_copy != NULL) {
		source = malloc(sizeof(char) * (strlen(to_copy)+1));
		source = strcpy(source, to_copy);
	} else {
		source = calloc(0, sizeof(char));
	}

	if (source[strlen(source)+1] != '\0') {
		source[strlen(source)+1] = '\0';
	}

	return source;
}

char *dstrcat(char *source, const char *append)
{
	if (source == NULL) {
		source = dstrcpy(source, NULL);
	}

	if (append != NULL) {
		source = realloc(source, sizeof(char) * (strlen(source)+strlen(append)+1));
		source = strcat(source, append);
	}

	return source;
}

char *dstrcatc(char *source, const char append)
{
	if (source == NULL) {
		source = dstrcpy(source, NULL);
	}

	if (append != '\0') {
		source = realloc(source, sizeof(char) * (strlen(source)+2));
		sprintf(source, "%s%c", source, append);
	}

	return source;
}

#endif

tests:

Code:
#include "assert.h"
#include "../dstr.h"

void main()
{
	char *copy_str = NULL;
	char *append_str = NULL;
	char ch = 'c';

	copy_str = dstrcpy(copy_str, "something");
	ec("something", copy_str);

	copy_str = dstrcpy(copy_str, "lorem");
	ec("lorem", copy_str);

	copy_str = dstrcpy(copy_str, NULL);
	ec("", copy_str);
	a(copy_str[strlen(copy_str)+1] == '\0', "copy_str doesnt have eof");

	append_str = dstrcat(append_str, NULL);
	ec("", append_str);
	a(append_str[strlen(append_str)+1] == '\0', "append_str doesnt have eof");

	append_str = dstrcat(append_str, "im");
	ec("im", append_str);

	append_str = dstrcat(append_str, " awesome");
	ec("im awesome", append_str);

	append_str = dstrcatc(append_str, ch);
	append_str = dstrcatc(append_str, ch);
	append_str = dstrcatc(append_str, '\t');
	ec("im awesomecc\t", append_str);
	a(append_str[strlen(append_str)+1] == '\0', "a doesnt have eof");

	free(copy_str);
	free(append_str);
}
 
Last edited:
Level 10
Joined
Sep 19, 2011
Messages
527
Code:
#include <stdlib.h>
#include <string.h>
#include <stdio.h>

#ifndef DSTR_H_INCLUDED
#define DSTR_H_INCLUDED

char *dstrcpy(char *source, const char *to_copy)
{
	size_t to_copy_len = 0;

	if (to_copy != NULL) {
		to_copy_len = strlen(to_copy);
	}

	if (source == NULL) {
		source = malloc(sizeof(char) * (to_copy_len+1));
	} else {
		source = realloc(source, sizeof(char) * (to_copy_len+1));
	}

	if (to_copy != NULL) {
		source = strcpy(source, to_copy);
	} else {
		source = strcpy(source, "");
	}

	return source;
}

char *dstrcat(char *source, const char *append)
{
	if (source == NULL) {
		source = dstrcpy(source, NULL);
	}

	if (append != NULL) {
		source = realloc(source, sizeof(char) * (strlen(source)+strlen(append)+1));
		source = strcat(source, append);
	}

	return source;
}

char *dstrcatc(char *source, const char append)
{
	size_t slen;

	if (source == NULL) {
		source = dstrcpy(source, NULL);
	}

	slen = strlen(source);

	if (append == '\0' && source[slen+1] == '\0') { // avoid multiple eof

	} else {
		source = realloc(source, sizeof(char) * (slen+2));
		sprintf(source, "%s%c", source, append);
	}

	return source;
}

#endif

What on earth is the point of any of that code? why are you reimplementing core functions in terrible ways...

dynamic char allocation.

Code:
char *s = NULL;

s = dstrcpy(s, "something with unkown length");
s = dstrcat(s, " keep setting");
s = dstrcatc(s, '\t');

if there is any core functions to do that i didn't noticed, could you link me to them pls?

thanks.

edit: is really that bad?, could you give me any advices or point me where you think its bad? thanks again.
 

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,255
On a side note, sizeof(char) is defined as 1. Always.
However for readability it might still be a good idea to use since it explicitly tells you that you are dealing with an array of char as opposed to some number of bytes.

When you see a malloc being passed an argument that is a multiple of "sizeof(char)" it is clear that you are dealing with an array of char. If you just see some quantity of bytes being allocated there is no readable attached purpose to the allocation so it could potentially lead to errors.

The code generated at the end should be no different as compilers mostly optimize out things like constant multiply by 1 so to use it or not is entirely a programming style choice.
 
Status
Not open for further replies.
Top