Reputation: 11
Here is the code I am having a problem with:
Struct:
struct AtoB
{
char * strA;
char * strB;
};
Function to create the struct. Called from external file.
AtoB * atob_create(char * a)
{
struct AtoB * atob = (struct AtoB *)malloc(sizeof(struct AtoB));
atob->strA = malloc(sizeof((char *)a));
strcpy(atob->strA, a);
atob->strB = NULL;
/* HERE IS PROBLEM NO 1 - SEGMENTATION ERROR OCCURS */
atob->strB = (char*)malloc(1);
strB = "\0";
for(int i = 0; i < (int)strlen(atob->strA); i++)
{
char token = atob->strA[i];
/*
:
append(AtoB * atob, const char a) MAY be called, so atob->strB &
atob->StrA will not be the same length
*/
append(atob, (char)token);
}
}
Function to append a char
to the char *
within the struct
. Called recursively.
void append(AtoB * atob, const char a)
{
size_t sz = strlen(atob->strB);
/* HERE IS ANOTHER ISSUE:
1: is this the right way to increase the size of the char * ?
2: does the struct also need to be realloc here to accommodate?
*/
atob->strB = (char*)realloc(atob->strB, sz + 1);
atob->strB[sz - 1 = a];
atob->[sz] = "\0";
}
Function to retrieve the final string. Called from external file.
char * (AtoB * atob)
{
return (atob->strB);
}
Function to free all the allocated memory. Called from external file.
void free(AtoB * atob)
{
free(atob->strA);
free(atob->strB);
free(atob);
}
As an exercise I am required to use C and take the content of one string, copying it in a different order or omitting or adding certain characters based on criteria that are not the issue here (therefore not included in the code).
I'm not having a problem creating the struct or allocating the first char * strA
. The problem arises when I try to allocate memory (even one char) to char * strB
.
I get a segmentation error.
It doesn't seem to matter how I try to do this, I have read up various examples and tried various ways and I just can't get it to work. I am aware there is some memory issue here but I can't seem to resolve it.
Even if I try to realloc
the struct I can't get strB
to be initialized to anything more than NULL
. I need to be able to append to atob->strB
at runtime, the final length of the string being different to the length of atob->strA
.
Following examples online, I thought this (as the most simple initialization) would work... it doesn't though:
atob->strB = NULL;
atob->strB = (char*)malloc(1);
atob->strB[0] "\0";
Is it something to do with the struct it's in?
I really can't get my head round what is wrong, have wasted much time searching for similar problems on various forums, reading round the topic and trying every single solution I can just trying to get past this first issue.
Now I'm just confused and out of time to solve this. If someone can tell me the actual code that I need, then I will be able to read it and understand what you did and why that is the correct method. I have commented in the areas where I am not clear and would really appreciate the correct solution.
Upvotes: 0
Views: 282
Reputation: 21319
When you initialize strB
to an empty string, you use:
strB = "\0";
But you should instead use either:
strB = '\0';
or:
strB[0] = '\0'.
The string literal "\0"
is an array of three chars
: {'\', '0', '\0'}
. You do the same thing in other places (here it looks like you left out the name of the struct
field):
atob->strB[sz] = "\0";
and (here I presume that you left out the assignment operator by accident):
atob->strB[0] = "\0";
There is an issue with the way that you grow the string. strlen()
gives the number of char
s in the array before the NUL
terminator, so if you want to add another char
you need to allocate for sz + 2
char
s. Then the new char
needs to be placed at strB[sz]
, which is where the old NUL
was, and NUL
needs to be placed at the end of the string, which is strB[sz + 1]
.
You can NUL
terminate the string strB
by changing your code to:
atob->strB = realloc(atob->strB, sz + 2);
atob->strB[sz] = a;
atob->strB[sz + 1] = '\0';
Also, it is possible for realloc()
to return NULL
, causing a memory leak. You should use a temporary variable to store the result of the call to realloc()
:
char *temp = realloc(atob->strB, sz + 2);
if (temp)
atob->strB = temp;
Upvotes: 0
Reputation: 3366
This code does not do what you think it does:
atob->strA = malloc(sizeof((char *)a));
^ sizeof(char *) is fixed. probably 4 or 8.
So, if a
points to a longer string, you will have a buffer overflow.
if you are using Linux, you better run valgrind memory checker to make sure you have no additional memory handling problems.
compile sure it is compiled with debug symbols (add -g to gcc command)
$ gcc -g -Wall -Werror a.c -o a.out
run it with gdb
$ gdb ./a.out
after the crash, look for backtrace
(gdb) bt
When you have it, just look where the crash is and try to understand why.
since we are dealing with memory handling problems, use valgrind:
$ valgrind --leak-check=full ./a.out
Look at the output and clean the errors.
Upvotes: 2