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

char *disemvowel(const char *str) {
    char vowels[] = { 'a', 'e', 'i', 'o', 'u', 'A', 'E', 'I', 'O', 'U' };
    int len = strlen(str), i, j, k = 0, flag;
    char *buf = malloc(sizeof(char) * len);
  
    for (i = 0; i < len; i  ) {
        flag = 1;
        for (j = 0; j < 10; j  )
            if (str[i] == vowels[j])
                flag = 0;
        if (flag) {
            buf[k] = str[i];
            k  ;
        }
    }
    buf[k] = '\0';
    buf = realloc(buf, strlen(buf));

    return buf;
}

The function correctly takes vowels out of input strings, but the tester code (from codewars) outputs this error:

The expression (as strings) (actual) == (expected) is false: 
actual=`dKwQMWGzMGtMFbvLJpnBRQXgZvRynrqVmzjklqgcpyngYdfWbKLhhZrhrTGFpdyPDNQNlnfgKWtRdGdcTqBWMkHKnCvpyLXqrVHKWlqfFxMqrkDjxNPmhtqFqvVCwNwqGftCHxnPRnPxyHgmdkTRnGtb1` 
expected=`dKwQMWGzMGtMFbvLJpnBRQXgZvRynrqVmzjklqgcpyngYdfWbKLhhZrhrTGFpdyPDNQNlnfgKWtRdGdcTqBWMkHKnCvpyLXqrVHKWlqfFxMqrkDjxNPmhtqFqvVCwNwqGftCHxnPRnPxyHgmdkTRnGtb`.

I don't know the input for this error, but the issue seems to be caused by the last character of the returned buffer. Any other input passed the test. Can someone point me to the right direction?

CodePudding user response：

... but the issue seems to be caused by the last character of the returned buffer.

At least this problem:

Off by 1

OP's code allocations do not account for space needed by the null character. strlen() reports a string's length, not including the null character. An allocation needs to account for the size of a string, which does include null character.

// char *buf = malloc(sizeof(char)*len);
char *buf = malloc(sizeof(char)*(len   1));

// buf = realloc(buf, strlen(buf));
buf = realloc(buf, strlen(buf)   1);

Other issues include:

• Not checking for allocation failure.

• Using int rather than size_t to handle very long strings.

• Inefficient, code already knows the length.

    buf[k] = '\0';
    // buf = realloc(buf, strlen(buf)   1);  // With corrected   1
    buf = realloc(buf, k   1);
  

• Inefficient: Once a vowel is found, no need to look for more,

    for (j = 0; j < 10; j  )
  
      // if (str[i] == vowels[j]) flag = 0;
      if (str[i] == vowels[j]) {
        flag = 0;
        break;
      } 
  

• Inconsistent to scale by size(char) in one allocation, but not the other.

CodePudding user response：

In addition to all the issues listed by chux, I spotted this one:

• potential memory leak: if realloc(buf, ...) fails, you return a null pointer and forget to free buf, which is no longer accessible. You should store the return value into another variable and return that or buf if realloc fails.

To avoid this and minimize fragmentation, you could first count the number of non vowels, then allocate the destination array, then iterate to copy the non vowels. This can be done efficiently with a lookup table:

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

char *disemvowel(const char *str) {
    static char const isvowel[(unsigned char)-1   1] = {
        ['A'] = 1, ['E'] = 1, ['I'] = 1, ['O'] = 1, ['U'] = 1, 
        ['a'] = 1, ['e'] = 1, ['i'] = 1, ['o'] = 1, ['u'] = 1,
    };
    size_t i, k;
    unsigned char c;
    for (i = k = 0; (c = str[i]) != '\0'; i  ) {
        k  = !isvowel[c];
    }
    char *buf = malloc(k   1);
    if (buf != NULL) {
        for (i = k = 0; (buf[k] = c = str[i]) != '\0'; i  ) {
            k  = !isvowel[c];
        }
    }
    return buf;
}