2

I am making a program that will search in an array of strings, and for each string, it will search for a specified char. If it finds that char, remove it. In this example I want to remove the character 'r'.

Here is the code:

void convertStrings(char **line) {
    for (int str = 0; str < MAX_LINE_LENGTH; ++str) {
        for (int ch = 0; ch < MAX_STR_LENGTH; ++ch) {
            if (line[str][ch] == 'r') {
                removeChar(line[str], 'r');
            }
        }
    }
}

void removeChar(char *str, char c) {
    int i = 0;
    int j = 0;

    while (str[i]) {
        if (str[i] != c) {
            str[j++] = str[i];
        }
        i++;
    }
    str[j]=0;
}

I am not sure if the algorithm for the removal of chars is correct, however the main mistake is elsewhere. More specifically, I get a segmentation fault in the line:

if (line[str][ch] == 'r') {

Why am I getting a seg fault? Also, is the algorithm for removeChar correct?

Here is my main function:

int main() {
    char line[3][10] = {"pep", "rol", "rak"};
    printf("%s\n", line[1]);
    convertStrings(line);
    printf("%s\n", line[1]);
    return 0;
}

Thanks in advance.

7
  • "Why am I getting a seg fault?" -- No idea. We'll have to see more code (Specifically, how do you call convertStrings and what are the values of MAX_LINE_LENGTH and MAX_STR_LENGTH). "Also, is the algorithm for removeChar correct?" -- Yes. It seems so. Commented Jun 2, 2015 at 14:56
  • are you sure line contains exactly MAX_LINE_LENGTH lines, and each string contains exactly MAX_STR_LENGTH chars? Commented Jun 2, 2015 at 14:56
  • Have you allocated enough memory to the object pointed to by the double pointer line? Commented Jun 2, 2015 at 14:57
  • I added my main function in the question. Commented Jun 2, 2015 at 14:59
  • @pk163 , What are the values of MAX_LINE_LENGTH and MAX_STR_LENGTH? Commented Jun 2, 2015 at 15:01

6 Answers 6

2

This code works on my compiler :

#include<stdio.h>
#include<conio.h>
#define MAX_LINE_LENGTH 1024
#define MAX_STR_LENGTH 4
void removeChar(char *str, char c) {
int i = 0;
int j = 0;

while (str[i]) {
    if (str[i] != c) {
        str[j++] = str[i];
      }
    i++;
}
str[j]=0;
}

void convertStrings(char line[][MAX_STR_LENGTH]) {    //change 1
 for (int str = 0; str < MAX_LINE_LENGTH; ++str) {
    for (int ch = 0; ch < MAX_STR_LENGTH; ++ch) {
        if (line[str][ch] == 'r') {
removeChar(line[str], 'r');
        }
    }
  }
}


int main() {
 char line[3][MAX_STR_LENGTH] = {"pep", "rol", "rak"};   //change 2
 printf("%s\n", line[1]);
 convertStrings(line);
 printf("%s\n", line[1]);
 getch();
 return 0;
}
Sign up to request clarification or add additional context in comments.

Comments

1

It's because line[str][ch] doesn't exist for all the value you give to str and/or ch.

You should check the value of MAX_LINE_LENGTH and MAX_STR_LENGTH and be sure that they are right.

Comments

1

The seg fault may be because you are using the constants "MAX_LINE_LENGTH" and "MAX_STR_LENGTH" however there may have the line length or string length. I would use the length of the array for the variable str in the first for loop instead of "MAX_LINE_LENGTH" and the length of array[str] instead of "MAX_STR_LENGTH". Unless each array you are searching has "MAX_LINE_LENGTH" and each string has "MAX_LINE_LENGTH" you will get a set fault. Hope this helps!

EDIT: you can find the length of the array by dividing the size of the array and the size of the type of the element.

sizeof(array)/sizeof(array[0])

finding the size of the char pointer is basically the same process.

1 Comment

Yes but what if not every element in the array has the same size?
1

You are getting a segfault either because array line contains fewer than MAX_LINE_LENGTH string pointers, or because at least one of the pointed-to strings contains fewer than MAX_STR_LENGTH characters; more likely the latter.

Instead of assuming a fixed number of strings of fixed length, you would be better off passing the actual number of strings as an argument. Alternatively, you could add NULL as sentinel value at the end of the list.

Moreover, there is no reason whatever to assume that each string is a fixed length. Look for the terminating character ('\0') to recognize when you've reached the end. For example:

void convertStrings(char **line) {
    for (char **l = line; *l != NULL; l += 1) {
        for (int ch = 0; (*l)[ch]; ch += 1) {
            if ((*l)[ch] == 'r') {
                removeChar(*l, 'r');
            }
        }
    }
}

Your removeChar() function looks ok.

Do note, however, that there are library functions that could help with this (e.g. strchr()), and that there are various efficiency improvements possible (such as passing to removeChar() only the string tail, starting at the first appearance of the character to remove).

Comments

1

You have the array

char line[3][10] = {"pep", "rol", "rak"};

When you pass it to a function, it gets converted into a pointer of type char(*)[10]. So change

void convertStrings(char **line) {

to

void convertStrings(char (*line)[10]) {

or

void convertStrings(char line[][10]) {

An array of arrays (2D array) cannot be converted to a pointer to a pointer(in this case, char**)


Another problem is that you mention that MAX_LINE_LENGTH is 1024 and MAX_STR_LENGTH is 4. This is wrong as the loop would iterate and you access invalid memory locations. You should make MAX_LINE_LENGTH as 3 and MAX_STR_LENGTH as 4 as there are 3 strings, each with 4 characters.
You can also pass these variables as parameters to the function convertStrings. Change add two more parameters in the declartion of convertStrings:

void convertStrings(char (*line)[10], int MAX_LINE_LENGTH, int MAX_STR_LENGTH) {

or

void convertStrings(char line[][10], int MAX_LINE_LENGTH, int MAX_STR_LENGTH) {

and call the function from main using

convertStrings(line, sizeof(line)/sizeof(*line), sizeof(*line)/sizeof(**line)); // `/sizeof(**line)` is 1 and is not needed


A better way would be to use

void convertStrings(int MAX_LINE_LENGTH, int MAX_STR_LENGTH, char line[][MAX_STR_LENGTH]) {

or

void convertStrings(int MAX_LINE_LENGTH, int MAX_STR_LENGTH, char (*line)[MAX_STR_LENGTH]) {

and call the function using

convertStrings(sizeof(line)/sizeof(*line), sizeof(*line)/sizeof(**line), line); // `/sizeof(**line)` is 1 and is not needed

so that you can avoid using the magic number 10 in your function.


You would've certainly got some warnings from your compiler. Pay attention to them. If you did not get warnings, crank up the warnings in your compiler and include warning flags ( like -Wall in GCC ).


BTW, You can look into the strchr function from string.h to find if a character exists in a string.

3 Comments

You are correct about 2D arrays vis a vis double pointers, and that certainly could explain the problem. It is to be hoped, however, that the OP did not ignore the compiler warning that surely would arise from such misuse, and then pose his question without even mentioning it.
@JohnBollinger , Added a note about warnings.
Would it be nicer to make the array of strings a typedef / struct so you need only define it once (and have a single-point of change if you alter the sizes etc.) then define the function as accepting a variable of that type (or a pointer to it)?
1

Why do you check if you encounter the 'r' character twice? (in both function) checking once would be enough.

A function to detect the char, and a function to delete it?

I would have done it this way :

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

void    convertStrings(char *line);
void    removeChar(char *str);

int     main(int argc, char *argv[]) {
    if (argc == 2)
    {
        printf("%s\n", argv[1]);
        convertStrings(argv[1]);
        printf("%s\n", argv[1]);
    }
    return (0);
}

void    convertStrings(char *line)
{
    for (int i = 0; line[i] != '\0'; i++)
    {
        if (line[i] == 'r') removeChar(&(line[i]));
    }
}

void    removeChar(char *str)
{
    int     i;

    i = 0;
    while (str[i] != '\0')
    {
        str[i] = str[i + 1];
        i++;
    }
}

But here is another one solution with only one function :

void convertStringsbis(char *line)
{
    int     delta;
    int     i;


    i = 0;
    delta = 0;
    while (line[i++ + delta] != '\0')
    {
        if (line[i + delta] == 'r')
            delta++;
        line[i] = line[i + delta];
    }
}

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.