CODEWITHSUNDEEP
carrayssegmentation-fault
geostocker
geostocker

Reputation: 1200

Unable to find the segmentation fault

What I need help understanding:

I'm trying to understand a segmentation error I'm getting:

Segmentation fault (core dumped)

From looking at SO and Google this seems to have to do with attempting to access memory the code doesn't have access to in some scope. However I can't seem to figure out where it's occurring.

What I'm trying to do (expecting):

I'm following along in The C Programming language and am attempting to solve exercise 2.4: squeeze(s1, s2) - remove all instances of characters in s2 in s1.

I have yet to touch on dynamic arrays etc, so I am only 'able' (in the mildest sense of the word) of working with simple, primitive data types. I've got experience with higher level languages though!

What I've done:

I'm running CygWin on a Windows 10 machine and have no issues with the compiler (gcc).

Here is the code I've written to solve the above problem:

/*
    Exercise 2.4
    squeeze (s1, s2): Remove all characters of s2 in s1.
    INPUT : s1.length >= s2 > 0.
    OUTPUT: The rest of s1 after deleting all occurances of letters in s2.
*/

#include <stdio.h>

void squeeze (char s1[], const char *s2);  /* Returns (by-ref) the resulting string s1 after removing all occurences of s2. */
int toUpper(char c);                      /* returns the numerical representation of a hexadecimal digit. */

int main () {
    char s1[] = "I am a test.\0";
    const char *s2 = "AE\0";
    printf("Before manipulation: %s\n", s1);
    squeeze(s1, s2);
    printf("After manipulation:  %s", s1);
}

/*
    Returns the (by-ref) resulting string s1 after removing all occurences
    of letters in s2.
*/

void squeeze (char s1[], const char *s2) {
    int index, s2_index, c = 0;

    while (s1[index] != '\0') {
        while(s2[s2_index] != '\0') {
            if ((c = toUpper(s1[index])) == s2[s2_index]){
                s1[index] = s1[index + 1];
            }
            s2_index++;
        }
        s2_index = 0;
        index++;
    }
}
/*
    Returns the upper-case representation of char c.
*/

int toUpper (char c) {
    if (c >= 'a' && c <= 'z')
        return c - 32;
    else
        return c;
}

Cheers! If you have any questions or if I've missed something, don't hesitate to comment! :)

Thanks for your help!

Upvotes: 0

Views: 102

Answers (2)

Lundin
Lundin

Reputation: 213276

Never write multiple variable declarations on a single line. This is considered very bad practice, as it makes it easier to write bugs. (Bad books might teach this style, beware of bad books.)

int index, s2_index, c = 0;

is identical to the more readable

int index;
int s2_index;
int c = 0;

As we can see from the readable version, you only initialize one variable to zero. Change the code to:

int index = 0;
int s2_index = 0;
int c = 0;

Better yet, you can de-obfuscate the loops into more readable code:

for(size_t i1=0; s1[i1] != '\0'; i1++)
{
  for(size_t i2=0; s2[i2] != '\0'; i2++)
  {
    ...
  }
}

Upon which you might note that you should probably just increase "index" each time you copy something, rather than at each lap in the loop(?).

Upvotes: 1

Marievi
Marievi

Reputation: 5001

You need to initialize index and s2_index.

Change :

int index, s2_index, c = 0;

to :

int index = 0, s2_index = 0, c = 0;

Upvotes: 1

Related Questions