2

im having a problem in my code, when i run it i got a segmentation fault in malloc() function. here's my code, im new here so sorry if i write something wrong.

Sorry my bad english !

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

typedef char String[50];
bool equalsIgnoreCase(char*,char*);

void main(){

 String nome;
 printf("Digite um nome: "); //name
 scanf("%s",nome);
  if(equalsIgnoreCase(nome,"TESTE")){ //test
     printf("Strings iguais."); 
  }else printf("Strings diferentes.");
}


bool equalsIgnoreCase(char *str1 , char *str2){
   char *a,*b;
   a = malloc(sizeof(char)); //segmentation fault here
   b = malloc(sizeof(char));
   for(;str1 != '\0';str1++,str2++){
     a = tolower(str1);
     b = tolower(str2);
     if(strcmp(a,b)!=0){
            free(a);
            free(b);
        return false;
    }
}
  free(a);
  free(b);
  return true;
}
Raphael Oliveira
  • 35
  • 4

3 Answers3

4

You shouldn't use malloc() and free() in this case. They are causing memory leak.

Also note that what is returned from tolower() is a character, not a pointer. Converting it to a pointer has little chance to yield a valid pointer.

Casting what is passed to tolower() from char to unsigned char is good because char may be signed and passing not in the range of unsigned char nor EOF to tolower() invokes undefined behavior.

Another point is that str1 != '\0' is not a correct way to tell if str1 is pointing at the end of string, and that you should also check for str2.

Finally, you should use const char* for strings that are not to be modified.

Your code should be like this:

bool equalsIgnoreCase(const char *str1 , const char *str2){
  for(;*str1 != '\0' && *str2 != '\0';str1++,str2++){
    if(tolower((unsigned char)*str1) != tolower((unsigned char)*str2)){
       return false;
    }
  }
  return true;
}

Or using a and b, like this:

bool equalsIgnoreCase(const char *str1 , const char *str2){
  int a, b;
  for(;*str1 != '\0' && *str2 != '\0';str1++,str2++){
    a = tolower((unsigned char)*str1);
    b = tolower((unsigned char)*str2);
    if(a != b){
       return false;
    }
  }
  return true;
}

Also don't forget to

  • change the prototype declaration of equalsIgnoreCase to bool equalsIgnoreCase(const char*,const char*); to match the new function.
  • add #include <ctype.h> to use tolower().
  • change the return type of main() to int to match the standard.
MikeCAT
  • 73,922
  • 11
  • 45
  • 70
0

There is plenty wrong on this code.

Let me start by saying that C already has a function to compare strings ignoring case-sensitivity, its called stricmp(), and works very much like strcmp() except it ignores case, you don't have to write your own.

Another important thing to point out is that a, b, str1 and str2 are all pointers.

When you do a = tolower(str1) you are not doing anything meaningful because str1 is not a character, its a pointer, and you are also losing the reference to the memory you have just allocated, and trying to free() memory in the stack which is invalid. One correct way of doing it, would be *a = tolower(*str1).

Also, the only string that fits in 1 char length is an empty one. Strings in C are null-terminated, which means you need 2 char to make a string with 1 character, because it has to be followed by \0 to be a valid C string.

Using malloc() here is also pointless, as pointed out on the other answer.

Community
  • 1
  • 1
Havenard
  • 27,022
  • 5
  • 36
  • 62
0

There has two bug in your code which may cause segmentation fault.

  1. for(;str1 != '\0';str1++,str2++){

compare str2 and '\0', or it will run out of str2.

  1. if(strcmp(a,b)!=0){

strcmp need char* and two bytes in this case, but a and b only has sizeof(char) which only one byte.

simple compare by *a != *b

at last, i fix the segmentation fault but the equalsIgnoreCase remain logical error, finish it by yourself. `

bool equalsIgnoreCase(char *str1 , char *str2){
   char *a,*b;
   a = (char*)malloc(sizeof(char)); //segmentation fault here
   b = (char*)malloc(sizeof(char));
   //for(;str1 != '\0';str1++,str2++){
   for(;*str1 != '\0' && *str2 != '\0';str1++,str2++){
     *a = (char)tolower((int)*str1);
     *b = (char)tolower((int)*str2);
     //if(strcmp(a,b)!=0){
     if (*a != *b) {
            free(a);
            free(b);
        return false;
    }
}
  free(a);
  free(b);
  return true;
}`
KunMing Xie
  • 1,613
  • 17
  • 15
  • 1. `str1 != '\0'` and `str2 != '\0'` won't be true before going out-of-range. Compare with what is pointed by them, not the pointers itself. 2. They say [you shouldn't cast the result of `malloc()` in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – MikeCAT May 03 '16 at 03:24
  • thank u for your advise. for 1, i had corrected it. for 2, cast malloc() to make compile success (VS2010 need cast) – KunMing Xie May 03 '16 at 03:30
  • I got the error, if any string is **larger** than another will return true, i fix that, thanks for the help. – Raphael Oliveira May 03 '16 at 15:08