-1

Possible Duplicate:
Problem with processing individual strings stored in an array of pointers to multiple strings in C

Ok so I'm trying to change a char of a string to another char in C. The thing is, each string is an element of a 1D array so essentially all together its a 2D array because a string itself is an array of chars. Anyways I have a problem creating code to do this. Is it even possible to do this? Any help is appreciated.

Here's the code:

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

int main ()
{ 
int i, size;
char **a;

a=(char**)malloc(sizeof(char*));

printf("Enter the size of the array:");
scanf("%d", &size);

for(i=0;i<size;i++){
 a[i]=(char*)malloc(sizeof(char)*8);
}

a[3]="Read";


while(*(a[3])!='\0'){
 if(*(a[3]) == 'e'){
    *(a[3]) = 'r';
 }
}

 printf("%s\n", a[3]);

 system("pause");
 return 0;

}
Community
  • 1
  • 1
user1311135
  • 23
  • 1
  • 4

5 Answers5

1
a=(char**)malloc(sizeof(char*));

printf("Enter the size of the array:");
scanf("%d", &size);

for(i=0;i<size;i++){
 a[i]=(char*)malloc(sizeof(char)*8);
}

Nope. You've allocated 1 char*. Then you treat it like it's size elements. You need to allocate size * sizeof(char*) bytes. (Note that this multiplication could also overflow.)

a[3]="Read";

Bad times. You are overwriting a[3] (which previously pointed to an allocation of 8 chars) with the location of a string literal, "Read". This leaks the previous allocation and also puts a non-modifiable string into a[3]. You should look into strncpy et al. for this.

asveikau
  • 39,039
  • 2
  • 53
  • 68
0

You didn't allocate enough space for a. Instead of

a=(char**)malloc(sizeof(char*));

you need

a=(char**)malloc(sizeof(char*)*size);

and obviously this must move to be after size has been read.

Once you sort that rather mundane problem out the fundamental problem is here:

a[3]="Read";

This makes the pointer a[3] point at a literal which cannot be modified. Instead you need to copy the contents of that literal into a[3]. Like this:

strcpy(a[3], "Read");

You must understand that a[3]=... assigns just the pointer a[3] and does not modify the string to which a[3] points.

Now, your code will obviously be in error if size is less than 4 since then a[3] would be out of bounds, but I guess a[3] is just transient while you debug this.

Your while loop is all wrong. Judging from your comments you want something like this:

char *p = a[3];
while (*p != '\0')
{
    if (*p == 'e')
        *p = 'r';
    p++;
}

No need to cast the return value of malloc in C, so remove the casts. sizeof(char) is always equal to 1 so you can remove that too.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • Ok so I get that I didn't allocate enough memory space for the array and its elements but I have another problem. I have no idea how to replace chars of the string stored in say a[3], that's why I tried using the while loop which doesn't work. – user1311135 Apr 03 '12 at 18:38
  • Contrary, you're not supposed to be able to change a literal, but you in fact can using a pointer. It's not like it goes into read-only memory. – std''OrgnlDave Apr 03 '12 at 18:39
  • Doesn't work is no good to us. How can we help if that's all the info you give? That said, I pointed out in my answer that your string assignment was wrong. Use strcpy instead. – David Heffernan Apr 03 '12 at 18:40
  • @OrgnlDave In fact, read-only memory is exactly where it goes, on typical systems. Modifying the contents of a literal is UB. – David Heffernan Apr 03 '12 at 18:40
  • I encounter an infinite while loop – user1311135 Apr 03 '12 at 18:44
  • That's because always test the same character and never set it to `\0`. What are you trying to achieve in that while loop? – David Heffernan Apr 03 '12 at 18:45
  • Ok so I forgot to increment the pointer(am I using the right pointer?). I was trying to read through the string until it reaches the null character and if it encounters the character e in the string it changes it to r – user1311135 Apr 03 '12 at 18:48
  • That's right. You need to introduce another pointer and use that to walk through `a[3]`. And don't forget to use `strcpy`! – David Heffernan Apr 03 '12 at 18:50
  • Ok thanks for the help! I'll figure it out now. – user1311135 Apr 03 '12 at 18:51
0

This:

a=(char**)malloc(sizeof(char*));

allocates space for one string. What you probably intend is something like:

char **a = NULL;
size_t number_of_strings = 8; /* for argument's sake */

a = malloc(number_of_strings * sizeof(char*));
if (!a) 
    return NOT_ENOUGH_MEMORY_ERROR;

At this point, you can dereference elements of a, e.g., a[3]. You'll still want to allocate space for those guys too:

char *staticStr = "Read";
a[3] = malloc(strlen(staticStr) + 1);
strncpy (a[3], staticStr, strlen(staticStr) + 1);

Start with that and see if rethinking how you're allocating memory will help you fix your other bugs.

Some notes:

  • You don't need to cast the result of malloc in C
  • You don't need to use sizeof(char) for allocating memory, which is always 1
  • You should be using a corresponding free() for each a[i] and for a itself, to prevent memory leaks
Alex Reynolds
  • 95,983
  • 54
  • 240
  • 345
0

I see another problem, when you allocate memory for the:

a = (char **) malloc(sizeof(char *));

You are allocating memory for just one position, but you are using size positions. Then your code should be:

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

int main ()
{ 
int i, size;
char **a;
char *ptr;



printf("Enter the size of the array:");
scanf("%d", &size);

 a=(char**)malloc(size * sizeof(char*));

for(i=0;i<size;i++){
 a[i]=(char*)malloc(sizeof(char)*8);
}

strcpy(a[3], "Read");

ptr=a[3];
while(*ptr!='\0'){
 if(*ptr == 'e'){
    *ptr = 'r';
 }
 ptr++;
}

 printf("%s\n", a[3]);

 system("pause");
 return 0;

}

and of course, you need to free the allocated memory.

In your while when you try to change the 'e' to 'r' you are always point to the same char. You need a new pointer to walk throw the array.

rbelli
  • 393
  • 1
  • 6
0

Your while loop doesn't do anything.

while(*(a[3])!='\0'){
 if(*(a[3]) == 'e'){
    *(a[3]) = 'r';
 }
}

It doesn't advance the pointer, it just keeps it at the first position.

A more proper-ish way would be to make a temporary pointer and use it to walk the string

char *temp = a[3];
while (*temp != '\0') {
  if (*temp == 'e') *temp = 'r';
  temp++;
}
std''OrgnlDave
  • 3,912
  • 1
  • 25
  • 34
  • I thought other people dealt with that pretty well. It doesn't matter, of course, because you *can* modify a literal. You shouldn't, you can't in debug mode, but it'll let you do it. – std''OrgnlDave Apr 03 '12 at 19:04
  • @DavidHeffernan I just built "char *a; a = "Hello!"; char *b = a; b[2] = 'b';" in Visual Studio 2010. Fault in debug mode, ran fine and even modified it in release. The proof is in the compiling. – std''OrgnlDave Apr 03 '12 at 19:18
  • @DavidHeffernan and I quote another StackOverflow comment, "Originally, the C89 (C90) standard did not outlaw modifying literals because there was too much code written before the standard that would be broken by it. Compilers can generate warnings. GCC 4.x does not have the -fwritable-strings option that GCC 3.x had, but GCC 3.x would warn about at least some attempts to modify strings. Writing const-correct code is harder than writing code which pays no attention to const, so people are apt to take the lazy route, still. " The standards, for backwards-compatibility, allow it – std''OrgnlDave Apr 03 '12 at 19:23
  • @DavidHeffernan I won't disagree that you shouldn't do it for 1,000 reasons. And also, in all C++ standards, they have never, ever been modifiable. But the fact remains that in C, you *can.* And other people already covered the topic of not modifying it. Should I copy-paste the answers into this? That's a real question - I'm not exactly an SO veteran. The real problem was worthless while loop - otherwise the code would've worked. – std''OrgnlDave Apr 03 '12 at 19:38
  • @DavidHeffernan I couldn't find an area of C99 that contradicted it, I'm not confusing anything. I wasn't spending a lot of time looking because I'm not asserting that you should do this or that it's even worth discussing. The point is you *can.* Have I hurt your pride or something? If it's in the C99 standard please cough it up, I'd like to expand my knowledge – std''OrgnlDave Apr 03 '12 at 19:46
  • @DavidHeffernan I asked about your pride because you're still arguing the point. Actually what is the point that we're arguing? – std''OrgnlDave Apr 03 '12 at 20:08
  • @DavidHeffernan past a certain point...certainly past C89, it doesn't matter if it's UB: if every compiler on the planet implements it that way, it's the standard. just like a dictionary doesn't give the standard for a language. here's a better example: In C++/anything bit shifts on signed integers are undefined, but every processor ever uses 2's complement so it's pretty well defined *by practice* if not by standard – std''OrgnlDave Apr 03 '12 at 20:22
  • K&R was the "standard" before C89. Anyhow, code did it before C89, and C89 didn't stop it. I'm not sure where you're going? Are you looking for a part of the C89 standard that says "modifying a literal is completely allowed because the memory structure works like x"? That may have been considered obvious at the time and not worth comment – std''OrgnlDave Apr 03 '12 at 20:32
  • @DavidHeffernan Grr, accidentally upvoted. Hate you can't un-upvote comments. I gotta go, I can't answer that question for you, sorry, if you find out the answer let me know – std''OrgnlDave Apr 03 '12 at 20:38
  • So, I asked the question: http://stackoverflow.com/questions/10001202/is-modification-of-string-literals-undefined-behaviour-according-to-the-c89-stan – David Heffernan Apr 03 '12 at 20:55
  • Please edit the relevant information into the question and/or answer, or even post new answers with new details if that is warranted. The comment section of a question/answer is only meant for relevant information regarding the actual post at hand, like mistakes, requests for more details, etc. not for long discussions even though they may be helpful to the people involved. If you need to continue this discussion, consider taking it to a chat room instead. – Lasse V. Karlsen Apr 03 '12 at 20:56