CODEWITHSUNDEEP
cstringchar
pavlos163
pavlos163

Reputation: 2890

Remove specified char from strings in C

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.

Upvotes: 2

Views: 8592

Answers (6)

vmonteco
vmonteco

Reputation: 15453

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

Upvotes: 1

Spikatrix
Spikatrix

Reputation: 20244

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.

Upvotes: 1

Sourav Kanta
Sourav Kanta

Reputation: 2757

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

Upvotes: 2

John Bollinger
John Bollinger

Reputation: 181714

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).

Upvotes: 1

coal175
coal175

Reputation: 273

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.

Upvotes: 1

Thomas Ayoub
Thomas Ayoub

Reputation: 29461

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.

Upvotes: 1

Related Questions