strstr will not return a true value

Question

I am working on this example in a C programming book and the strstr command is supposed to trigger the printf command when the value is true. It is trying to find a string within tracks and return which track it was found in. I have palyed around with this for over an hour and cant seem to find what is wrong. Currently it is not printing anything even when there is supposed to be a match.

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

char tracks[][80] = {
        "Boston",
        "Where the Streets Have No Name",
        "Row Row Row your Boat",
        "Gangsta Paradise",
        "Yoda",
    };


void findTrack(char searchFor[]){
    int i;
    for(i = 0; i < 5; i++){
        if(strstr(tracks[i], searchFor))
            printf("Track %i: '%s'\n", i, tracks[i]);

    }
}

int main(){

    char searchFor[80];
    printf("what is your string?: ");
    fgets(searchFor, 80, stdin);
    printf("searching for: %s", searchFor);
    findTrack(searchFor);

    return 0;
}

`strstr` doesn't return true or false. It returns a pointer. — chris, Jul 19 '14 at 01:48
`printf("searching for: %s", searchFor);` is this working? what is the output? — bumbumpaw, Jul 19 '14 at 01:49
you will need to remove the newline(`'\n'`) at the end of the line from `searchFor`. — BLUEPIXY, Jul 19 '14 at 01:51
@chris Are you arguing pedantic semantics or do you just not know what you're talking about? A non-null value is true in C; strstr will return non-NULL, and therefore true, and therefore the `if` condition will be satisfied, if the substring is found in the string. — Jim Balter, Jul 19 '14 at 02:43
@JimBalter, Of course it can be used like a boolean (in fact, I prefer not explicitly testing pointers against null), but to me, the question sounds as if a boolean result is expected. — chris, Jul 19 '14 at 02:49
@chris Ok, so you don't know what you're talking about. Again, in C, non-zero is true and zero is false. And simply looking at the question and the code makes it obvious that's what the OP understands, and wants, since the value of strstr is only being tested for 0/non-0 in an if statement. — Jim Balter, Jul 19 '14 at 02:51
@JimBalter, Yes, a non-zero value is true and a zero value is false. I'm not arguing that. To me, the question looks like the usage was taken from the book without understanding the specifics of what goes on to make it happen. I could be completely wrong about that. — chris, Jul 19 '14 at 02:54
@chris You obviously are. Again, the OP is using strstr correctly and asking the correct question about it. The problem is that `searchfor` contains a newline ... there's nothing wrong with the usage or understanding of `strstr` here. Finally: "I'm not arguing that" is false, because your initial comment was "strstr doesn't return true or false", which is simply wrong ... again, non-0 is true, 0 is false. — Jim Balter, Jul 19 '14 at 03:02
@JimBalter, I know the usage is completely correct. My whole argument is based on *when the value is true*. "value" seems most fit for the result of `strstr`, so "true" is out of place, which suggests to me that there could be a lack of understanding in the type being returned by `strstr`. It's easy to find an example of how to use a function and infer things from it incorrectly. — chris, Jul 19 '14 at 03:08
@chris "true" is not out of place. Again, *in C*, non-0 is true, 0 is false. Thus, strstr returns true or false depending on whether the substring is found, and that is how it is *usually* used ... people rarely use the pointer value. What's "nonsense" here is people who refuse to admit error. "which suggests to me that there could be a lack of understanding in the type being returned by strstr" -- all you have to do is *look at the code*, and the discussion below, to see that there is no such lack of understanding. Again, the problem is the newline, not the usage of strstr. — Jim Balter, Jul 19 '14 at 03:10
@JimBalter, If we incorporate C99, true would most accurately be reserved for `bool`. Even then, non-zero and zero still fit the bill. What I disagree on is the *is*. Pointers are pointers. By themselves, they are not intrinsically true or false, but perhaps I'm taking too much of a C++ approach with `bool` being a type and conversions to it specified and everything. — chris, Jul 19 '14 at 03:14
@chris "If we incorporate C99, true would most accurately be reserved for bool." No, that is completely wrong. `true` isn't even available if stdbool.h isn't included, but we aren't talking about *the symbol* `true`. *In all versions of C*, true is non-0 and false is 0 ... that's what all boolean contexts, in if/while/for, &&, || ?: test for. "Pointers are pointers." -- And cats are felines, but they're also mammals. "By themselves, they are not intrinsically true or false" -- YES, THEY ARE. NULL is intrinsically false and non-NULL is intrinsically true in C (and C++ too). — Jim Balter, Jul 19 '14 at 03:18
@chris "perhaps I'm taking too much" -- that was the other option for what you're doing. If you insist on pointless pedantics, take the question to be "strstr will not return a value that, when converted to bool, is true" (or "is true in boolean contexts"). That is **obviously** what was meant. Sheesh. I'm done here, and a bit disgusted. — Jim Balter, Jul 19 '14 at 03:23
The discussion at http://stackoverflow.com/q/3825668/1967396 discusses this question of `!=NULL` in some more depth. I think it shows this is a matter of personal style - with different 100k+ users arguing for opposite points of view. So follow the style guide if your organization uses one, and pick your own style and stick to it if they don't. — Floris, Jul 19 '14 at 11:46

score 3 · Accepted Answer · edited Jul 19 '14 at 02:42

3

I recommend that you print out the search string as entered - you will probably find there is a newline character at the end of the line. You should include newlines in the strings to search for, or strip it from the input.

You might do this as follows:

char *p = strchr(searchFor, '\n');
if (p!=NULL) *p='\0';

Further, I would recommend that for "clean" code you should write

if(strstr(tracks[i], searchFor)!=NULL) {

this shows clearly that you understand that strstr returns a pointer.

edited Jul 19 '14 at 02:42

M.M

138,810
21
208
365

answered Jul 19 '14 at 02:00

Floris

45,857
6
70
122

I tried this but it still doesnt return anything. if i invert the if condition it prints all the strings like i would expect, but when the strings are compared it breaks. – Jpm61704 Jul 19 '14 at 02:14
Try changing your line from `printf("searching for: %s", searchFor);` to `printf("searching for: #%s#", searchFor);` - I think you will find there is an extra carriage return on the input. – Floris Jul 19 '14 at 02:19
So it did make a new line unexpectadely, how do i remove this newline? – Jpm61704 Jul 19 '14 at 02:22
The cleanest way is to use `strchr` to find the newline character and write `'\0'` in its place. I will update the answer... – Floris Jul 19 '14 at 02:24
Is this a common thing with fgets? i changed it to scanf and that seemed to fix the problem – Jpm61704 Jul 19 '14 at 02:26
Yes it is a common thing with `fgets` - you get the whole line including the carriage return. – Floris Jul 19 '14 at 02:27
Thanks Floris, I appreciate all the help :) – Jpm61704 Jul 19 '14 at 02:30
Even cleaner: `if((strstr(tracks[i], searchFor)!=NULL)!=0)`, because it shows clearly that you understand that `!=` evaluates to an `int`. Sorry, I couldn't resist… ;) – mafso Jul 19 '14 at 02:59
That isn't "cleaner", it is unreadable garbage. There is no need to make the code more verbose to show an understanding that strstr returns a pointer ... what it returns is irrelevant as long as its true if `searchFor` was found and false it it wasn't, which is the case and the OP clearly understands that. – Jim Balter Jul 19 '14 at 03:14

score 2 · Answer 2 · edited May 10 '20 at 14:21

2

Complementing @floris answer...

A neat trick to convert a pointer to boolean is to double negate the pointer (!!pointer), like this:

if(!!strstr(tracks[i], searchFor))
    printf("Track %i: '%s'\n", i, tracks[i]);

edited May 10 '20 at 14:21

Ardent Coder

3,777
9
27
53

answered May 10 '20 at 14:19

Clayton A. Alves

388
2
10

score 0 · Answer 3 · edited Oct 17 '20 at 18:50

0

It's because of fgets() will add a new line character to the input. So the SerchFor input will become "SearchFor\n" (the input). To compare, you have to remove the new line character.

Here is the code:

int len = strlen(buffer);

if(buffer[len-1]=='\n')
   buffer[len-1]='\0';

you can add this code after the fgets().

edited Oct 17 '20 at 18:50

fcdt

2,371
5
14
26

answered Oct 17 '20 at 17:21

Nirmal Johnson

1
1

2

1) `fgets()` does not add a `'\n'`. It retains the `'\n'` if it was read into the buffer. 2) Corner case: `buffer[len-1]` fails (UB) when the first character read is a _null character_. – chux - Reinstate Monica Oct 17 '20 at 23:12
[`buffer[ strcspn( buffer, "\n" ) ] = '\0'`](https://stackoverflow.com/a/28462221/4756299) – Andrew Henle Oct 21 '22 at 22:48

strstr will not return a true value

3 Answers3