CODEWITHSUNDEEP
carraysstringfunction
Michael Tesla
Michael Tesla

Reputation: 11

Passing an array of characters as function arguments

I am trying to pass a string as an argument to a function and it throws a Segmentation fault(Core Dumped) error. Could you please let me know what mistake I am making here? Here is the code:

replaceChars(char str[], char sChar[], char rChar)
{
int i,j;
printf("rChar is %c", rChar);
printf("sChar is %s", sChar);

for(i = 0; i <= strlen(str); i++)
{
    for(j = 0; j <= strlen(sChar); j++)
    {
     if(str[i] == sChar[j])  
     {
        str[i] = rChar;
        //printf("The New String is %c", str[i]);
     }
    }
}

printf("The New String is %s", str);
}
void main()
{
char myString[36], schar[36], rchar;

printf("Please enter a string:");

scanf("%[^\n]s", &myString);

printf("Which characters to replace?");
scanf(" %[^\n]c", &schar);
printf("With which character?");
scanf(" %[^\n]c", &rchar);

replaceChars(myString, schar, rchar);

}

Upvotes: 1

Views: 73

Answers (2)

chux
chux

Reputation: 154242

Could you please let me know what mistake I am making here?

In addition to @dbush good answer, OP's code is unnecessarily inefficient.

Using the corrected code below, and assume the initial length of the str, sChar are S,C respectively.

for(i = 0; i < strlen(str); i++) {
  for(j = 0; j < strlen(sChar); j++) {
    if(str[i] == sChar[j]) {
      str[i] = rChar;
    }
  }
}

The for(i = 0; i < strlen(str); i++) { and with the later str[i] = rChar; obliges the code to find the length of str up to S times and each strlen(str) requires O(S) operations.

If S was a non-trivial value, say 1000, this 1000*1000 could readily affect overall performance. A simply solution is to calculate the length once or look for the null character instead.

// for(i = 0; i < strlen(str); i++) {
S = strlen(str);
for(i = 0; i < S; i++) {
// or
for(i = 0; str[i]; i++) {

The same thing happens with the inner loop too. Yet a smart compiler can see that sChar does not change and may take advantage of understanding strlen() has no side effects that would cause for a different result. With such an optimization strlen(sChar) may be truly called once, even if strlen(sChar) in inside the higher for (i...) loop.

Still it is easy and idiomatic to just test for the null character.

    // for(j = 0; j < strlen(sChar); j++)
    // better as
    for(j = 0; sChar[j]; j++)

Yet why does this not apply to the for(i = 0; i < strlen(str); i++) loop?

Within that loop, code can modify str[] and so the compiler cannot make the optimization as with for(j = 0; sChar[j]; j++).

This also begs the question, what should code do if the replacement character rChar is the null character?

As I see it, code could either continue, replacing with a '\0 multiple times or simple return after this first.

       str[i] = rChar;
       if (rChar == '\0') return; // Possible way to handle \0

Upvotes: 0

dbush
dbush

Reputation: 225362

Two issues here.

First, when you loop through str and sChar:

I am trying to pass a string as an argument to a function and it throws a Segmentation fault(Core Dumped) error. Could you please let me know what mistake I am making here? Here is the code:

for(i = 0; i <= strlen(str); i++)
{
    for(j = 0; j <= strlen(sChar); j++)
    {

You use <= as your exit condition. Array indexes start from 0, so valid indexes are from 0 to length-1. You're going from 0 to length, so you're stepping of the end of the array. Reading past the end of an array invokes undefined behavior.

Change the conditions to use <:

for(i = 0; i < strlen(str); i++)
{
    for(j = 0; j < strlen(sChar); j++)
    {

The second problem is in how you're reading the values:

scanf("%[^\n]s", &myString);
...
scanf(" %[^\n]c", &schar);
...
scanf(" %[^\n]c", &rchar);

The %[] format specifier doesn't require any characters after it, and it requires a char * as a parameter which points to the first element of an array of char. In the first two cases, you're passing the address of an array instead of the array itself (which decays to a pointer) and you have an extra character after the %[] format that isn't needed. In the third case you pass a pointer to a single char when a pointer to multiple characters is expected by the format. Because you want to read a single char, you want to use the %c format specifier instead.

scanf("%35[^\n]", myString);
...
scanf(" %35[^\n]", schar);
...
scanf(" %c", &rchar);

Note also that the first two have a field width that limits the number of characters that are read so that you don't overrun the arrays.

Upvotes: 2

Related Questions