0

This is Leetcode's palindrome problem, which I solved with JavaScript. Then I decided to try it with C and the problem I'm running into is that if I define the array by value, when I would try to return a pointer to it, it would show up as NULL. After some research on here, I found that declaring an array would put it on the stack instead of the heap and that space in memory is reclaimed when the function closes, so I was returning an undefined spot in memory.

The problem I have is that I can't seem to create a character array using malloc. I don't know if it's the syntax I'm getting wrong or something I'm missing. It's been a while since I used C, so I'd appreciate any help.

char * longestPalindrome(char * s) {
    int strLength=strlen(s);  
        
    /* char longestPal[strLength]; */ 
    /* works as far as the logic is concerned but can't return a pointer to it */
       
    /* the trouble spot I need help with */
    char *longestPal = malloc(sizeof(*char) * strLength);  
    /* compiler error */
        
    for (int i = 0; i <strLength; i++) {
        /* first check for longest even palindrome */
        findPalindrome(i, i, strLength, longestPal, s); 
                
        /* Next check for longest odd palindrome */
        findPalindrome(i, i + 1, strLength, longestPal, s); 
    }
     
    return longestPal;  
}
    
void findPalindrome (int left, int right, int strLength, char *longestPal, char *s) { 
    int i, j;
         
    while (left >= 0 && right < strLength && s[left--] == s[right++])
        if ((right - left) > strlen(longestPal)) {
            j = 0;
            for (i = left + 1; i < right; i++) {
                longestPal[j] = s[i];
                j++;
            }
        }

    return;     
}

Edit After giving up on allocating memory, I went in a slightly different direction and decided to work within the already allocated memory in the parameter *s. I admit I was looking for inspiration from another solution and changed my "middle-out" palindrome finder function to just give me the length of the palindromes and I would use that to determine the longest and then use that to return the address to the correct part of the palindrome.

char * longestPalindrome(char * s){
    int strLength=strlen(s); /*doing this so I don't keep calling strlen and waste resources*/
    int start=0, end=0;
    int tempMax1, tempMax2, max;
    int i=0;
    
    if (s==NULL || strLength<1)
        return "";
    
    for(i=0; i<strLength; i++)
        {
        
            /*first check for longest even palindrome*/
            tempMax1 = findPalindrome(i,i, strLength, s); 
            
            /*Next check for longest odd palindrome*/
            tempMax2= findPalindrome(i, i+1,strLength, s); 
            
            /*keep track of the longest palindrome found*/
            max= tempMax1 > tempMax2? tempMax1:tempMax2;
            
            if (max > end-start)
            {
                start = i-(max-1)/2;
                end= i+max/2;
            }
            
        }
    /*we need a way to indicate the end of the palindrome
    * easiest way is to mark the end with the NULL character
    */
    
    s[end+1]='\0';
    return &s[start];

   
}

/*will return the length of the palindromes it finds
* and then the maximum will be determined by the main function
*/
int findPalindrome (int left, int right, int strLength, char *s)
{ 
     

    while (left>=0 && right < strLength && s[left]==s[right])
    {
        left--;
        right++;
    }
    
    return (right-left-1);
  
}
  • `sizeof(*char) * strLength` should be `sizeof(char) * (strLength+1)`. Note the two changes. Don't forget to allocate a byte for the trailing 0 that ends C strings. Also, you might want to learn about the newish `std:string` to avoid manually dealing with this. And don't forget you have to call `free` when you `malloc` – user2740650 Jan 03 '22 at 03:07
  • 2
    @user2740650: The *newish std:string* isn't available in C. – Ken White Jan 03 '22 at 03:09
  • Whoops, you're right, my bad. I was thinking of C++. – user2740650 Jan 03 '22 at 03:10
  • [this](https://stackoverflow.com/questions/41830461/allocating-string-with-malloc) should be helpful, but note that SO consensus is do [_not_ cast the return value of `malloc`](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). Also `sizeof char` is defined to be 1, so some omit it. – yano Jan 03 '22 at 03:33
  • Thank you so much for all your help! I decided to go ahead and use the memory already allocated in *s and instead of creating new string arrays and copying them into *s, to just use the indexes of the palindrome and return a part of the string. Kind of stole that idea off someone else but the idea was brilliant...and I changed a few things they did. The finished product is in the bottom and it passes. – No-name-yet Jan 25 '22 at 03:37

1 Answers1

0

You dont want the extra * in char *longestPal = malloc(sizeof(*char) * strLength); . You also should add 1 to include a byte for the null terminator at the end of the string.

Change it to char *longestPal = malloc(sizeof(char) * strLength + 1);.

I'd be weary of allocating the memory inside the function like this, however. If free is never called on this buffer later, the program will have a memory leak. It would make more sense to have the caller allocate the memory for this, and pass in a pointer to that memory buffer to longestPalindrome(), along with the size in bytes.

Also, currently you are detecting the desired length from the line int strLength=strlen(s);, which most likely is not doing what you think. strlen searches for the first null character in the char buffer passed in to determine the length, so unless you are passing in a string (a char * array) that is already populated with non-null characters for the desired length, then strLength will not be correct. I don't see any benefit in doing it this way, so I doubt that's how you are expecting it to work. If you are going to stick to allocating the string memory inside the function, then you don't need to pass in a char* as the argument to longestPalindrome, you should just pass the desired string length.

Edits:

Also add 1 in call to malloc to include spot for null terminator.

Sam
  • 109
  • 5
  • 1
    `(strLength+1)` for the trailing 0 byte. – user2740650 Jan 03 '22 at 03:54
  • `malloc(sizeof(char) * strLength);` won't be enough for a NUL byte. I also disagree with the rest of your assessment. It's perfectly good practice for functions to accept strings, allocate memory, and return pointers to that memory, just as long as everything is documented properly. `strlen` for instance accepts a `char*` that's assumed to point to a valid C string .. or invokes undefined behavior if it doesn't. – yano Jan 03 '22 at 03:57
  • @yano Edited my answer to include the null terminator byte allocation. Thank you. I'm sticking with my guns in terms of the rest of my assessment. I agree that with good documentation you could implement it this way, but most good APIs will not allocate memory needlessly. The caller knows how long the string needs to be, it can allocate the space. Also, it surely is weird to pass in a string when the only thing used from the string is the length of it. Will it work? yes. Is it good practice? no. – Sam Jan 03 '22 at 13:39
  • Thank you all for your answers. Unfortunately, being Leetcode, I don't have control over the calling function so I can't allocate the memory there. I made the changes to my malloc but still get the error below. I will try putting the code in visual studio and see if it works. ================================================================= ==31==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000036 at pc 0x7f1210d7fa6d bp 0x7ffe4776fb60 sp 0x7ffe4776f308. – No-name-yet Jan 04 '22 at 04:12
  • since *s already had its memory allocated by the calling function, can I somehow use it as my final array? Perhaps copy its content locally, do what I need to do to the local file and then modify the contents of *s ? – No-name-yet Jan 04 '22 at 04:27
  • @No-name-yet yes you can definitely do that. Then if you still need a temp char array you can just declare it on the stack as you were doing before, and just copy the result to the s char array at the end before returning. Another thing I'm seeing which may be causing you issue is the use of the `strlen` function on `longestPal` before it is initialized. `strlen` works by finding the first NULL character to determine string length. Since this is uninitialized, there is no guarantee where the first null character will be found, so it's not going to return the proper length of that array. – Sam Jan 04 '22 at 15:45