1

I'm getting Segmentation Fault from this code, Can anyone tell me what's wrong? I'm getting a Segmentation Fault so I assume I messed up the pointers at some point. It should return and print the greatest number in the string.

It should return number 5.

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

int* greatest(char* string); 
int main() 
{   
   char *string = "This4 is5a 3wonderful2 day";
   int *p = 0;   
   p = greatest(string);
   printf("%d\n",*p);
}

int* greatest(char* string)
{
   int i = 0;
   int *poi = 0;
   while (*(string + i) != '\0')  
   {
       if(*(string + i) >= '0' && *(string + i) <= '9')
     {
       if(*((string + i)-48) > *poi)
       {
         *poi = *((string + i)-48);
       }
       }
     i++;
   }
   return poi;
 } 

This is what I'm getting after executing the code:

" Segmentation fault (core dumped)"

Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
Mike Mikey
  • 11
  • 1
  • 2
    `int *poi = 0;` sets the pointer to `NULL` and then you dereference it - disaster! – Weather Vane Aug 27 '18 at 19:02
  • `int *poi = 0;` There's no need for this to be a pointer anyway. – 001 Aug 27 '18 at 19:03
  • i had a question to return a pointer, so i thought that was how to do it – Mike Mikey Aug 27 '18 at 19:05
  • But you never set `poi` to anything else. Remember/compare the greatest value in/with another variable, and whenever exceeded set `poi = string + i`. But `poi` and the function must be `char*`. – Weather Vane Aug 27 '18 at 19:06
  • `*((string + i)-48)` ouch. If you use a more conventional form of language to access the members of this array of characters, you would be less likely to experience that pain: `string[i]-48` or even better `string[i]-'0'` – Tim Randall Aug 27 '18 at 19:09
  • i'm still 2 months into this i'm trying to learn by my self – Mike Mikey Aug 27 '18 at 19:12
  • 1
    Sorry if I came across as condescending Mike. Do you see what the code is doing? It takes the address of the first character in your string, adds `i`, subtracts 48, and reads from that address. – Tim Randall Aug 27 '18 at 19:14

2 Answers2

1

The main issue is that you set int *poi = 0; and then try to put a value into that location. You can't do that. A pointer has to point to a memory location before you can store a value in it. 0 or NULL are invalid addresses used to mark a pointer as uninitialized. (The pointer points to nowhere.)

To make it a valid pointer, make it point to something:

int *poi;
int p;

poi = &p;
*poi = 123;

(You can also use malloc to dynamically allocate memory for a pointer).

I'm guessing you are supposed to return a char * and that should point to the address of one of the chars in the string:

char* greatest(const char* string)
{
   // Safety check
   if (NULL == string) return NULL;

   char *poi = NULL;
   while (*string)  // Loop until string points to end-of-string ('\0')
   {
       if (*string >= '0' && *string <= '9') { // see also: isdigit
           if (NULL == poi)  { // poi hasn't been assigned yet
               poi = string;
           }
           // No need to convert to int. Can just compare char codes
           else if (*string > *poi) {
               poi = string;
           }
       }
       string++;
    }

    // At this point, poi either points to a char in string,
    // or NULL (if no digits in string)
    return poi;
}

Then change main to:

int main() 
{   
   const char *string = "This4 is5a 3wonderful2 day";
   char *p = greatest(string);
   // Check p before printing it
   if (NULL == p) {
       printf("No digits in: %s\n", string);
   }
   else {
       printf("%c\n", *p);
   }
}
001
  • 13,291
  • 5
  • 35
  • 66
0
#include <stdio.h>
#include <string.h>



int greatest(char* string);
int main()
{
  char *string = "This4 is5a 3wonderful2 day";
  int p = 0 ;
  p = greatest(string);
  printf("%d\n",p);

}

int greatest(char* string)
{
  int i = 0;
  int poi = 0 ;
  while (string[i] != '\0')
  {  
    if((string[i]) >= '0' && *(string + i) <= '9')
    {
      if(((string[i])-'0') > poi)
      {
       poi = ((string [i])-'0');
      }  
    }
    i++;
  }
  return poi;
}

i guess i have to do like this. i wanted to return a pointer but that's fine if i don't find the answer for it, i will keep trying. Also thanks for the help, Amazing website with amazing people

Mike Mikey
  • 11
  • 1