1

I was messing around with all of the string functions today and while most worked as expected, especially because I stopped trying to modify literals (sigh), there is one warning and oddity I can't seem to fix.

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

int main() {
char array[] = "Longword";
char *string = "Short";

strcpy(array, string); // Short

strcat(array, " "); // Short (with whitespace)

strcat(array, string); // Short Short

strtok(array, " "); // Short

if (strcmp(array, string) == 0)
{
    printf("They are the same!\n");
}

char *substring = "or";

if (strstr(array, substring) != NULL)
{
    printf("There's a needle in there somewhere!\n");
    char *needle = strstr(array, substring);
    int len = strlen(needle);
    needle[len] = "\0"; // <------------------------------------------------
    printf("Found it! There ya go: %s",needle);
}

printf("%s\n", array);

return 0;
}

Feel free to ignore the first few operations - I left them in because they modified array in a way, that made the strstr function useful to begin with.

The point in question is the second if statement, line 32 if you were to copy it in an editor. (EDIT: Added arrow to the line. Sorry about that!)

Pumamori
  • 75
  • 6
  • Don't make us all count, which is line 32? – Barmar Jan 14 '16 at 00:25
  • 2
    `needle` is pointing to some offset into `array[]` the `array[]` is already NUL terminated by the strcat() function, so this line: `needle[len] = "\0";` is completely not needed. – user3629249 Jan 14 '16 at 00:31
  • Even if I wanted to just output "or"? I thought it would output "ort" if I didn't add the \0. – Pumamori Jan 14 '16 at 00:33
  • 1
    *Feel free to ignore the first few operations* Sorry, cannot do that. this line: `strcat(array, string);` is trying append a string onto array, resulting in undefined behaviour, due to overflow of the array[] which can lead to a seg fault event. – user3629249 Jan 14 '16 at 00:34
  • If that's what you want, you need to do `needle[len-1] = '\0';` – Barmar Jan 14 '16 at 00:34
  • Or maybe you meant to use `int len = strlen(substring);` – Barmar Jan 14 '16 at 00:36
  • 1
    why call `strstr()` twice, just save the result from the first call. similar to: `if ( (char* needle = strstr(array, substring) ) != NULL)` – user3629249 Jan 14 '16 at 00:38
  • The first one still outputs "ortShort", not sure on how to go about the second one with only the adress of the first "or". `needle[strlen(needle)-strlen(needle-2)] = '\0';` is what I tried now, and it outputs "ort" which comes close, but the second I use -3, it goes back to ortShort. – Pumamori Jan 14 '16 at 00:41
  • this line has a problem: `needle[len] = "\0";` the problem is `"\0" is a string not a character. suggest: `needle[len] = '\0';` – user3629249 Jan 14 '16 at 00:42
  • this line has a problem: `int len = strlen(needle);`, the returned type from `strlen()` is `size_t`, not `int`. suggest: `size_t len = strlen(needle);` which will eliminate the -Wconversion warning and will not require any other changes to the code. Note: the implicit conversion feature of C will result in the correct values being generated, but why produce code that contains hidden problems? – user3629249 Jan 14 '16 at 00:45
  • 1
    As user3629249 hints at, after correcting the '\0' character, your code, `int len = strlen(needle); needle[len] = '\0';` is not doing what you think it is doing. The `strlen(needle)` is _not_ returning the length of the substring (as you intended). `needle` points to "ort" in `array`, so `strlen(needle)` returns 3, not the 2 you wanted, and you're simply storing a NUL character in `array` where there already is one. Use `needle[strlen(substring)] = '\0';` to NUL-terminate the substring in `array`. (And fix the other problems, particularly `strcat()` writing past the end of `array`.) – Alex Measday Jan 14 '16 at 02:24

2 Answers2

4

This line is wrong:

needle[len] = "\0";

Doublequotes make a string literal, whose type is char *. But needle[len] is a char. To make a char literal you use singlequotes:

needle[len] = '\0';

See Single quotes vs. double quotes in C or C++

Community
  • 1
  • 1
Barmar
  • 741,623
  • 53
  • 500
  • 612
  • Thank you for your quick answer! To think it was that easy. The output I get from the statement now is `ortShort` though. Is that because I corrupted the original array? Could I fix it by making the size of the original bigger? – Pumamori Jan 14 '16 at 00:30
  • 1
    Most likely. Try `char array[100] = "Longword";` – Barmar Jan 14 '16 at 00:35
2

Your second strcat call overruns the end of array, corrupting whatever happens to be after it in memory. Once that happens, the later code might do just about anything, which is why writing past the end of an array is undefined behavior

Chris Dodd
  • 119,907
  • 13
  • 134
  • 226