3

I'm using with a smaller piece of code to test functionality for a larger (beginner) program, but I don't understand the difference between two strings.

I found and used:

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

int main()
{

char *string, *found;

string = strdup ("1/2/3");
printf("Orig: '%s'\n",string);

while ((found = strsep(&string,"/")) != NULL )
  printf ("%s\n",found);

return (0);
}

and this print the tokens one at a time.

Then when I try and move to a user entered string:

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

int main()
{
  char string[13],
  char *found, *cp = string;

  fprintf(stderr, "\nEnter string: ");
  scanf("%12s",string);
  printf("Original string: '%s'\n",string);

  while((found =  strsep(&cp,"/,-")) != NULL )
    printf("%s\n",found);

  return(0);
}

I get a seg fault. I understand the basics of pointers, arrays and strings, but clearly I'm missing something, and would love for someone to tell me what it is!

Also - if I change printf("%s\n",found); to printf("%i\n",found); I get some junk integers returned, but always the correct amount, e.g. If I enter 1/2/3 I get three lines of integers, 1111/2222 I get two lines.

Thanks!

-Edit- There was an adittional problem with strsep, detailed here. Thanks all.

Weaver
  • 145
  • 8
  • You need to allocate space for the user-defined string. The string literal knows how long it is. – JGroven Mar 20 '18 at 17:17
  • Seem like there is a guy mass-downvoting without telling why. Cool. This will help to improv us (because I don't understand why I was downvoted). – Tom's Mar 20 '18 at 17:28
  • Thanks for all the answers. I remember reading about this so just wrapping my head around it. In the meantime, I tried everyone's suggestion of fixing the size but I still get a seg fault! Updating question to reflect this. – Weaver Mar 20 '18 at 17:39
  • Also, I've not voted on anyone's answer yet so I'll be sure to do it once I've understood everything. Hopefully it'll help counter the downvoter! – Weaver Mar 20 '18 at 17:40
  • Update - I changed the specifier in printf to `printf("%p\n",found);` and I get three addresses output so strsep id doing it's work. – Weaver Mar 20 '18 at 18:40

7 Answers7

4

In the first piece of code, string is assigned the return value of strdup, which allocates space for the string to duplicate and returns a pointer to that allocated space.

In the second piece of code, string uninitialized when it is passed to scanf, so scanf is reading the invalid value in that pointer and attempting to dereference it. This invokes undefined behavior which in this case manifests as a crash.

You need to set aside space for the user's string. A simple way to do this is to create an array of a given size:

char string[80];

Then tell scanf how many characters it can read in:

 scanf("%79s",string);
dbush
  • 205,898
  • 23
  • 218
  • 273
  • Thanks, a very nicely presented answer that reminded me of how pointers work. For some reason the string won't print, even after correcting the length of `string`! – Weaver Mar 20 '18 at 19:15
  • @William If you have `cp` declared as a `char *` it should work. Your example currently has it as `char`. – dbush Mar 20 '18 at 19:17
  • Thanks so much for your time and input. No, it was my typo in the question, I had the `*` in my code. For clarity I think I'll start a new question regarding this other problem, and keep this for the original mistake. – Weaver Mar 20 '18 at 21:04
2

Differences between the two cases:

  1. In the first case string points to valid memory that was allocated by strdup while in the second case you are trying to read into invalid memory.

  2. The first case is well behaved while the second case is cause for undefined behavior.

The second case can be fixed by allocating memory for it using malloc or using a fixed size array.

char *string,*found;
string = malloc(100); // Make it large enough for your need.
fprintf(stderr, "\nEnter string: ");
scanf("%99s",string);

or

char string[100], *found;
fprintf(stderr, "\nEnter string: ");
scanf("%99s",string);

Make sure you deallocate dynamically allocated memory. Otherwise, your program leaks memory.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • Thanks, I remember now the pointer goes to whatever was in the memory. Strangely I still get seg faults even after allocating the length. – Weaver Mar 20 '18 at 18:11
  • You are not passing the correct argument to `strsep`. Use `char* cp = string; while((found = strsep(&cp,"/,-")) != NULL )`. – R Sahu Mar 20 '18 at 18:16
  • Ah yes, I see that! The code now progresses past the strsep (I put a simple `printf` line in after it) but seg faults on the `printf("%s\n",found);` line. There's something I'm still not spotting! – Weaver Mar 20 '18 at 18:25
  • @William, I can't explain why that's happening. – R Sahu Mar 20 '18 at 18:30
1

You should allocate memory for the user input string.

First option is statically

char string[256];

and second option is dynamically using malloc() function 

char *string;

string = (char*) malloc(256 * sizeof(char));
if (string == NULL)
   {
   //error
   }

Don't forget at the end to release the allocated memory

free(string);
chanioxaris
  • 946
  • 10
  • 9
  • what's the purpose of doing a malloc like that ? You can just type "char string[256]" and the result will be the same. – Tom's Mar 20 '18 at 17:20
  • It's just a second way to allocate memory. What's wrong with that? – chanioxaris Mar 20 '18 at 17:22
  • and in the original code the first string is mallocd by strdup. – pm100 Mar 20 '18 at 17:23
  • Because in your example, you seem to free it rigth after. So what's the purpose of this allocation ? It add complexity to the code, for nothing. But I didn't read that you put the statically form, sorry. (and sizeof(char) == 1, so no need to add it, and if you really want a sizeof, it's better to have "sizeof(*string) ) – Tom's Mar 20 '18 at 17:27
  • You are right about sizeof(char) is equal to 1, just to be typical and not risk to confuse the OP. I added the free() to remember put it at the end of the code to release the allocated memory – chanioxaris Mar 20 '18 at 17:35
0

You didn't allocate the space needed ! You have to have a memory space to write to. You can have it statically "char string[256]", or dynamically with allocation.

In your first example, you use "strdup" that does a malloc, but scanf will not allocate memory for you.

If you want all the user input, you usually wrap the scanf in a while loop in order to retrieve the user input chunk by chunk. Then you have to reallocate each time your buffer is insuffisante to add the chunk.

If you just want to retrieve a string from stdin without doing any format-checking, I strongly recommand fgets.

Tom's
  • 2,448
  • 10
  • 22
0

The reason is very simple. Your string variable is a char pointer and you need to allocate memory to it to store a string.Probably in your first case strdup ("1/2/3"); returns a string and your char pointer *string points to the string return by strdup function and that is the reason why you are not getting the segmentation error in the first case. Even in your first case also you might get a segmentation error if enter a very long string.

so allocate enough memory to the string pointer like below in your second example and that will fix your problem:-

 char *string = malloc(50);// here malloc will allocate 50 bytes from heap
Abhijit Pritam Dutta
  • 5,521
  • 2
  • 11
  • 17
0

In user enter string case you do not have memory allocated to the pointer string.In the first case, strdup is allocating memory for string pointer while in the second case you do not have any memory associated with string pointer leading to segfault. first, allocate memory using malloc and then use scanf.

chetan kankotiya
  • 106
  • 6
0

char string[13] char cp=string

Here cp is a variable of type char and as 1 byte of memory allocated

It won't be able to store a char array of 13 character which would be 13 bytes, and it's because of this you are getting segmentation fault

Joy Mathews
  • 21
  • 2
  • Ok, that's interesting, I thought char *cp was a pointer? When I change `printf("%s\n",found);` argument to `%i` I get (random) integers, and it return the correct number of tokens I am expecting so I thought the whole string was being passed correctly – Weaver Mar 20 '18 at 18:59