0

Function locates the last occurrence of ch in the string pointed to by s. It returns a pointer to the character, or a null pointer if ch does not exist in the string. I'm trying to write the function without using string library functions.

This is what I got so far, it seems right to me, but I can't seem to get the resultant string.

#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h> 
#include <string.h> 
#include <math.h> 

char *strrchr2(char *s, char ch);

int main()
{
    char str1[100];
    char char1;
    char result_str[100];

    printf("\nEnter a string: ");
    gets(str1);
    fflush(stdin);
    printf("Enter the target char in the string: ");
    scanf("%c", &char1);
    char * result_str = strrchr2(str1, char1);
    printf("Resultant string = %s", result_str);

char * strrchr2(char *s, char ch)
{
    int count = 0, offset = 0;
    while (*(s + count) != '\0')
    {
        if (*(s + count) == ch)
            offset = count;
        count++;
    }
    return *(s + offset);
}

Expected output:

Enter a string: abcdefdfdfghh
Enter the target char in the string: f
Resultant string: fghh
unintendedjoy
  • 281
  • 2
  • 10
  • 18
  • 3
    You are declaring `result_str` twice !! – Arjun Sreedharan Mar 19 '14 at 08:30
  • 2
    Your compiler should issue an error at line `return *(s + offset);`. Suche as `cannot convert from 'char' to 'char *'` – Jabberwocky Mar 19 '14 at 08:33
  • 1
    `fflush(stdin)` is undefined behaviour. `fflush(stdout)` is fine, though – Elias Van Ootegem Mar 19 '14 at 08:39
  • @EliasVanOotegem Since he/she obviously using MSVC with that `#define _CRT_SECURE_NO_WARNINGS`, `fflush(stdin);` is *defined behaviour*, with the definition that can be found over there: http://msdn.microsoft.com/en-us/library/9yky46tz.aspx (check the Remarks and the Example sections) – Utkan Gezer Mar 19 '14 at 08:44
  • @ThoAppelsin Microsoft do not override the C standard and MSVC is not a conforming compiler, so who cares. You should use a conforming C compiler for compiling C code. – Lundin Mar 19 '14 at 09:04
  • @ThoAppelsin: The comment above the example they give there is quite clear: _`// fflush on input stream is an extension to the C standard`_. Using compiler-specific extensions is sometimes fine, but in this case, there is no need. Besides, MSDN contains examples of `malloc` and `calloc` where the `void *` is cast, too. Though required in C++, that's actually considered bad practice in C. Using a _"true"_ C compiler is still worth while. – Elias Van Ootegem Mar 19 '14 at 09:06
  • @EliasVanOotegem Yeah, the comment above the example is directly what I was referring to. That line clearly defines that MSVC compiler will do so. I did not say anything more than that. `fflush(stdin);` *does have a defined behaviour* for MSVC compiler. – Utkan Gezer Mar 19 '14 at 18:30
  • @Lundin I also have opinions, many does. – Utkan Gezer Mar 19 '14 at 18:31

3 Answers3

3
return *(s + offset);

You are returning here the character at s[offset]. You have to return the pointer to this location which is (s + offset)

return (s + offset);
Sakthi Kumar
  • 3,047
  • 15
  • 28
  • ...once that's fixed, you still fail to return a NULL pointer if the character does not exist in the string – mfro Mar 19 '14 at 08:51
  • @mfro I'm not really sure how to return a NULL pointer, can I just put *s = '\0' then return s in the else part? – unintendedjoy Mar 19 '14 at 09:42
  • @unintendedjoy you're heading in the wrong direction. – zoska Mar 19 '14 at 10:01
  • a NULL pointer is a pointer variable with the _value_ (void *) 0L. You would thus just do a "return NULL". This obviously requires the calling code to explicitely check the return value for NULL and act accordingly - otherwise you will run into trouble. *s = '\0' is a pointer to NUL (yes, this is no typo). This is something completely different. – mfro Mar 19 '14 at 13:53
2
const char* strchr_last (const char* s, char ch)
{
  const char* found_at = NULL;

  while(*s != '\0')
  {
    if(*s == ch)
    {
      found_at = s;
    }
    s++;
  }

  return found_at;
}
Lundin
  • 195,001
  • 40
  • 254
  • 396
1

You can do the same as with finding the first occurrence of a character in the string, with a little change: scanning the string from the end to the start.

char* strrchr2(char *s, char ch)
{
    char* p = s;
    int found_ch = 0;
    //finding the length of the string
    while (*p != '\0')
    {
        p++;
    }
    //p now points to the last cell in the string
    //finding the first occurrence of ch in s from the end:
    while (p >= s && !found_ch)
    {
        if (*p == ch)
        {
            found_ch = 1;
        }
        else
        {
            p--;
        }
    }
    if (!found_ch)
    {
        p = 0;
    }
    return p;
}
alextikh
  • 119
  • 1
  • 9
  • 1
    It is probably better to just iterate through the string once, as in my answer. In the worst case scenario where the letter searched for only sits in the very beginning of the string, your code will have to iterate through the whole string twice: first forward and then backward. The only case where this algorithm performs better is when the searched for symbol is always expected to be at the end of the string. – Lundin Mar 19 '14 at 10:24