Segmentation fault in a well compiled C program

Question

I am solving a problem on USACO. In the problem, I have to take two strings as inputs and calculate the numerical values modulo 47. If the values are same, then GO is to be printed otherwise STAY has to be printed. The initial numerical value will be calculated by taking the product of the numerical values of the alphabets ( 1 for A and similarily 26 for Z ) and then the final number will be calculated by using modulo.

My program is being compiled withour any error and running well on my computer. However, it is showing a segmentation fault as the execution error by the grader computer. The program and the output is as follows:-

Program:-

#include<stdio.h>
#include<string.h>
main()
{
    int cal(char *ptr);
    char *comet,*group;
    int a,b;
    scanf("%s",comet);
    a=cal(comet);
    scanf("%s",group);
    b=cal(group);
    if(a==b)
        printf("GO");
    else
        printf("STAY");
    return 0;
}
int cal(char *ptr)
{
    int i=0,c,prod=1,mod;
    while(ptr[i]!='\0')
        {
            if(ptr[i]>='A'&&ptr[i]<='Z')
            {
                c=ptr[i]-'@';
                prod=prod*c;
                i++;
            }
        }
    mod=prod%47;
    return mod;
}

OUTPUT:-

enter image description here

My question is how to pinpoint the segmentation fault. I have read about this fault but don't know what to do in this program. Any help would be great.

You can't use the value of a variable until you assign that variable a value. — David Schwartz, Feb 02 '13 at 01:54

score 1 · Accepted Answer · answered Feb 02 '13 at 01:22

1

char *comet,*group;
int a,b;
scanf("%s",comet);

comet pointer is uninitialized. You need to allocate memory and makes comet points at this allocated memory otherwise scanf will write bytes in a random location which will likely crash your system.

answered Feb 02 '13 at 01:22

ouah

142,963
15
272
331

Alex Reynolds · Answer 2 · 2013-02-02T07:31:42.193

1

You never allocate space for comet or group. Use malloc() or similar to set aside memory for those pointers, so that you can actually store something in what they point to.

#define MAX_STRING_LENGTH 256

...

char *comet, *group;
int a, b;

comet = NULL;
comet = malloc(MAX_STRING_LENGTH);
if (!comet) {
   fprintf(stderr, "ERROR: Could not allocate memory to comet\n");
   return EXIT_FAILURE;
}
scanf("%s",comet);

/* repeat for other pointers, as needed */

/* ... */

/* free up allocated memory at the end of the program to help prevent leaks */

free(comet);
comet = NULL;

edited Feb 02 '13 at 07:31

answered Feb 02 '13 at 01:26

Alex Reynolds

95,983
54
240
345

You want to check if the value is still `NULL` after `malloc()` because you might not get the space you ask for. Granted, this is a trivial example, but for more complicated programs, developing good memory management habits is important. – Alex Reynolds Feb 02 '13 at 02:09

score 1 · Answer 3 · edited May 23 '17 at 12:18

Both comet and group are uninitialized pointers which do not have any memory allocated for storing the input strings.

Your program should be doing this at least. Increase the size of MAX_STRING_SIZE per your needs.

#define MAX_STRING_SIZE 100
char comet[MAX_STRING_SIZE];
char group[MAX_STRING_SIZE];

You still have the risk of buffer overflow with scanf. You can look at this post for some possible ways to avoid buffer overflow.

score 1 · Answer 4 · answered Feb 02 '13 at 01:51

Your while is highly suspect: i is increased only if ptr[i] is an uppercase letter. What should happen if it isn't? How does If you have an ironclad guarantee that only uppercase letters will show up, you could write:

prod = 1;
while(*ptr) {
  prod *= *ptr - 'A' + 1;
  ptr++;
}

(Your ptr[i] - '@' had me scratching my head until I broke out ascii(7). I believe my version is clearer, and any halfway competent compiler will give the same code.) Or, more idiomatically:

int cal(char *ptr)
{
    int prod = 1;

    while(*ptr)
       prod *= *ptr++ - 'A' + 1;
    return prod % 47;
}

Just be careful that the product does't overflow, perhaps do the modulus each character:

int cal(char *ptr)
{
    int prod = 1;

    while(*ptr) {
       prod *= *ptr++ - 'A' + 1;
       prod %= 47;
    }
    return prod;
}

abhi · Answer 5 · 2013-02-02T02:21:57.880

"running well on my computer" this is not possible in your case as you are not using any compiler specific code (like getch for turbo c)

You didn't allocate memory for storing the string.The pointers comet and group don't point to anything.scanf requires an address to write the input but the pointers do not contain an address and that is why you are getting a segmentation fault. You can allocate memory using malloc (or calloc) or you can define a character array.

The corrected code is

#include<stdio.h>
#include<string.h>
#define MAXLENGTH 100
int main()
{
    int cal(char *ptr);
    char comet[MAXLENGTH],group[MAXLENGTH];
    int a,b;
    scanf("%s",comet);
    a=cal(comet);
    scanf("%s",group);
    b=cal(group);
    if(a==b)
        printf("GO");
    else
        printf("STAY");
    return 0;
}
int cal(char *ptr)
{
    int i=0,c,prod=1,mod;
    while(ptr[i]!='\0')
    {
        if(ptr[i]>='A'&&ptr[i]<='Z')
        {
            c=ptr[i]-'@';
            prod=prod*c;
            i++;
        }
    }
    mod=prod%47;
    return mod;
}

Segmentation fault in a well compiled C program

5 Answers5