CODEWITHSUNDEEP
c
kusur
kusur

Reputation: 618

Segmentation fault in a well compiled C program

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.

Upvotes: 1

Views: 211

Answers (5)

Alex Reynolds
Alex Reynolds

Reputation: 96937

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;

Upvotes: 1

abhi
abhi

Reputation: 71

"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;
}

Upvotes: 0

vonbrand
vonbrand

Reputation: 11791

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;
}

Upvotes: 1

Tuxdude
Tuxdude

Reputation: 49473

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.

Upvotes: 1

ouah
ouah

Reputation: 145829

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.

Upvotes: 1

Related Questions