0

I have this code in C, where I will be inputting a string of numbers separated by spaces, and splitting it with strsep. If I input a string like "1 2", and set strcmp to look for a number before the last element, the code works, but if I set strcmp to look for the last element, the code fails. Can you suggest any fixes?

char *string = malloc(1028), *found;
  if (fgets(string, 1028, stdin) != NULL) {
    while ((found = strsep(&string, " ")) != NULL) {
      if (strcmp(found, "2") == 0) {
        printf("%s\n", found);
      }
    }
  }
  • Please provide a [minimal verifiable example](https://stackoverflow.com/help/minimal-reproducible-example) of the failing case. Are you saying it doesn't work as expected if `"2"` is used in `strcmp`? If that is the case then show that code not the code that works. Note that an MVE is complete code that can be run exactly as shown to reproduce the problem. – kaylum Oct 13 '20 at 05:32
  • That's because your last element is `"2\n"` not `"2"`. Use `found[strcspn (found, "\n")] = 0;` before `strcmp()`. – David C. Rankin Oct 13 '20 at 05:36
  • One possible problem is that the string actually has a newline at the end. – kaylum Oct 13 '20 at 05:36

1 Answers1

2

It's because the last element that found points to is including the new line character. So you'd either have to add the new line to strcmp like so: strcmp(found, "2\n") (assuming 2 was your last element) or when you call strsep you need to tokenize on both the space and new line character: strsep(&string, " \n").

Complete solution:

char *string = malloc(1028);
if (string) {
  if (fgets(string, 1028, stdin) != NULL) {
    char *p = string, *found = string;
    while ((found = strsep(&p, " \n")) != NULL) {
      if (strcmp(found, "1") == 0) {
        printf("%s\n", found);
      }
    }
  }
  free(string);
}

A few things to observe: The string pointer is modified by the strsep function so I've updated the code to use an intermediate variable p. I've also updated the code to validate the allocation was successful and then free it at the end. Depending upon your system requirements, you could consider forgoing the heap and allocating string on the stack.

hgs3
  • 525
  • 3
  • 6
  • 1
    You will want to explain you MUST preserve a pointer to the beginning of `string` BEFORE `strsep()` is called. Otherwise the pointer to the beginning of the allocated block is lost creating a memory leak. Better `char *string = malloc(1028);` (validate the allocation), then `char *p = string, *found = string;` and use `found = strsep (&p, " \n");` so `free (string);` can be called when done. See [man 3 strsep](https://www.man7.org/linux/man-pages/man3/strsep.3.html) – David C. Rankin Oct 13 '20 at 05:41
  • @DavidC.Rankin Good suggestions, I've updated my answer. – hgs3 Oct 13 '20 at 16:08