-1

I'm diving into pointers and strings in C and I'm still getting used to some concepts. I tried to implement a version of the strchr() function – the same as in string.h – for study purposes, but something basic is still not right.

Here's my code:

#include <stdio.h>

char* my_strchr(const char* str, int c){
  if (str == NULL){
    printf("STR is NULL. Finishing the program\n");
    return NULL;
  }
  while (*str != '\0'){
    if (*str == c){
      return (char*) str;
    }
    str++;
  }
  return NULL;
}

int main(){
  char *a = "Hello World!";
  char *b;
  char c;

  printf("Type the character you want to find in the Hello World! string:\n");
  scanf(" %c", &c);

  b = my_strchr(a, c);

  printf("Character found! %c\n", *b);

  return 0;
}

I'm trying to figure out why this is returning a segmentation error. When I use gbd, it tells me that the error is in the last printf, which tries to print the *b.

Once my_strchr() returns a (char*) str, I'd have to store this return value in a char pointer variable, right?

Jongware
  • 22,200
  • 8
  • 54
  • 100
ulissesBR
  • 93
  • 11
  • 3
    Does this happen with all input, or only with characters that do not appear in your test string? – Jongware Dec 28 '16 at 14:41
  • @Rad Lexus with all input... Still trying to figure out what is happening... – ulissesBR Dec 28 '16 at 14:42
  • 2
    Show your input. Also `strchr` can search `'\0'`. – BLUEPIXY Dec 28 '16 at 14:43
  • yes your code works fine for me – user2736738 Dec 28 '16 at 14:44
  • 4
    Not with "all input". Entering `e` works. But when not found, you don't test the return value before deferencing `NULL`. – Weather Vane Dec 28 '16 at 14:45
  • 1
    I just tried. It *does* work, as long as you do not search for a character not in your string. If you do, you are attempting to print the contents of `NULL`. – Jongware Dec 28 '16 at 14:45
  • @BLUEPIXY: interesting – it's even [mentioned](http://en.cppreference.com/w/c/string/byte/strchr). Perhaps inverting the test is all that needed to add that functionality. – Jongware Dec 28 '16 at 14:48
  • @BLUEPIXY any input will produce the error. I'm working in a Debian running in an Oracle VM VirtualBox. Once my code worked fine for coderredoc I have no idea if this environment can have something to do with this fault. – ulissesBR Dec 28 '16 at 14:49
  • Have you got Caps Lock on? – Weather Vane Dec 28 '16 at 14:49
  • @WeatherVane I'll try to work on this. But anyway, even if I type H the fault happens... Very strange! – ulissesBR Dec 28 '16 at 14:50
  • Guys, thanks a lot... BLUEPIXY and WeatherVane, the problem is really trying to print the NULL contents. I'll work on it! – ulissesBR Dec 28 '16 at 14:52
  • Guys, thank you so much, I just added an if statement that tests if b == NULL and now works like a charm. Thanks a lot for everything! – ulissesBR Dec 28 '16 at 14:56

2 Answers2

5

When my_strchr doesn't find the character in the string, it returns NULL.

In this case b is NULL so *b is undefined behavior, which explains the segfault.

You might want to check the result of my_strchr before printing *b, e.g.:

if (b != NULL) {
  printf("Character found! %c\n", *b);
} else {
  printf("Not found...\n");
}
Emil Laine
  • 41,598
  • 9
  • 101
  • 157
  • thanks a lot for your answer, this was the problem! I solved it adding an if statement to test if b is NULL and now works like a charm! Thanks everyone! – ulissesBR Dec 28 '16 at 14:55
0

There are some logic issue like tuple_cat said.

But I also think you don't understand some concept, your code is not clean from my point of view.

I guess you just started coding in c so keep coding :)

First you return a char* in your function but you define argument of the function as

char* my_strchr(const char* str, int c)

In standard C you can't touch a constant you can't modify it that's the point of declaring a constant.

so change the function to

char* my_strchr(char* str, int c)

Then the correct way to return a char from a string is not

return (char*)str;

but just

return str;

At the end of your function.

This way you will send the address of the first char in char* (string). In a char* you do that by just giving the variable name.

I encourage you to read: https://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html

RTFM !!! the char* part at 1.3.4 String Constants

Anyway good luck in your learning.

  • 1
    Thanks @Raphael, I'll dig on it. Anyway, the reference I was following tells that, when passing a string as an argument, function argument should be a constant to avoid any modifications in the string. – ulissesBR Dec 28 '16 at 15:40
  • That's right, if you do not modify it. but then what's the point of returning a char* ?If you don't do anything with it you could just check like it does something that's ok then returning. void or else but yeah it's just to avoid compiler Warnings it's recommendation. –  Dec 28 '16 at 15:45
  • 3
    This is just bad advice concerning `const`-correctness. You should read the function prototype for the standard library version: `strchr(const char *string, int c)`. In this case, the `const` indicates that the value stored at the location pointed to by `string` can not be modified; but the value of the pointer itself can be modified (to loop through the string, for example). When emulating library functions, it is best to stick the the standard prototypes and to try to understand why they are written that way. OP was doing this. – ad absurdum Dec 28 '16 at 19:20
  • 1
    Also, I recommend reading the [C Standard](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf) for reference. The standard does discuss library requirements. You should also read the manual for whatever standard library implementation you are using. For gcc, here is the [manual for glibc](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf). – ad absurdum Dec 28 '16 at 19:26
  • @DavidBowling I did what it was supose to do without compiler warnings,but thanks for the advice I will also check. –  Dec 29 '16 at 13:29