0

I am trying to write code to implement strchr function in c. But, I'm not able to return the string.

I have seen discussions on how to return string but I'm not getting desired output

const char* stchr(const char *,char);
int main()
{
    char *string[50],*p;
    char ch;
    printf("Enter a sentence\n");
    gets(string);
    printf("Enter the character from which sentence should be printed\n");
    scanf("%c",&ch);
    p=stchr(string,ch);
    printf("\nThe sentence from %c is %s",ch,p);
}
const char* stchr(const char *string,char ch)
{
    int i=0,count=0;
    while(string[i]!='\0'&&count==0)
    {
        if(string[i++]==ch)
            count++;
    }
    if(count!=0)
    {
        char *temp[50];
        int size=(strlen(string)-i+1);
        strncpy(temp,string+i-1,size);
        temp[strlen(temp)+1]='\0';
        printf("%s",temp);
       return (char*)temp;
    }
    else
        return 0;
}

I should get the output similar to strchr function but output is as follows

Enter a sentence
i love cooking
Enter the character from which sentence should be printed
l
The sentence from l is (null)
11
  • 2
    Use a debugger to step through the code. It will show you where your logic is wrong. Commented Oct 28, 2019 at 10:34
  • 2
    Learning how to debug is an important part of learning how to program. Commented Oct 28, 2019 at 10:34
  • 2
    Why are you doing all those things in the stchr function? All that strchr does it finding the char in question and then returning the address of that char. No need for this suspect dynamic memory routine. Unless your strch shouldn't actually be a strchr implementation and is supposed to do something else that you didn't mention. Commented Oct 28, 2019 at 10:37
  • 3
    1) string, in main function is actually an array of char pointers (not a string). char string[50]; is the correct declaration. 2) as someone mentioned above, your strchr() has a strange funtionallity: it counts the number of ch occurrencies in string and returns a brand new string instead of providing the address of ch first occurrency. 3) If you want a function returning a copy of the original string, you cant return the address of a local variable. It's contained in thaa stack, and it will be overwritten after the function return. Commented Oct 28, 2019 at 10:47
  • Don't use gets: stackoverflow.com/questions/1694036/… Commented Oct 28, 2019 at 11:21

1 Answer 1

1

There are basically only two real errors in your code, plus one line that, IMHO, should certainly be changed. Here are the errors, with the solutions:

(1) As noted in the comments, the line:

char *string[50],*p;

is declaring string as an array of 50 character pointers, whereas you just want an array of 50 characters. Use this, instead:

char string[50], *p;

(2) There are two problems with the line: char *temp[50]; First, as noted in (1), your are declaring an array of character pointers, not an array of characters. Second, as this is a locally-defined ('automatic') variable, it will be deleted when the function exits, so your p variable in main will point to some memory that has been deleted. To fix this, you can declare the (local) variable as static, which means it will remain fixed in memory (but see the added footnote on the use of static variables):

static char temp[50];

Lastly, again as mentioned in the comments, you should not be using the gets function, as this is now obsolete (although some compilers still support it). Instead, you should use the fgets function, and use stdin as the 'source file':

fgets(string, 49, stdin);/// gets()  has been removed! Here, 2nd argument is max length.

Another minor issue is your use of the strlen and strncpy functions. The former actually returns a value of type size_t (always an unsigned integral type) not int (always signed); the latter uses such a size_t type as its final argument. So, you should have this line, instead of what you currently have:

size_t size = (strlen(string) - i + 1);

Feel free to ask for further clarification and/or explanation.

EDIT: Potential Problem when using the static Solution

As noted in the comments by Basya, the use of static data can cause issues that can be hard to track down when developing programs that have multiple threads: if two different threads try to access the data at the same time, you will get (at best) a "data race" and, more likely, difficult-to-trace unexpected behaviour. A better way, in such circumstances, is to dynamically allocate memory for the variable from the "heap," using the standard malloc function (defined in <stdlib.h> - be sure to #include this header):

char* temp = malloc(50);

If you use this approach, be sure to release the memory when you're done with it, using the free() function. In your example, this would be at the end of main:

free(p);
Sign up to request clarification or add additional context in comments.

5 Comments

note that using a static array means the function is not thread safe.
@Basya Noted, but for this 'simple' int main() program, that will not be an issue. Of course, using char *temp = malloc(50); is safer (and actually better, IMHO), but I try to keep things uncomplicated for early-stage learners of the language. My suggestion will solve (and hopefully explain) the issue.
Fair, but I am concerned that someone might copy this and not realize its implications.
Cool. That got you an upvote :-) -- the rest of the answer is good information.
I didn't realize that my code has these silly mistakes and errors. Thanks for helping me realizing them...

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.