Returning a pointer through the function argument, but losing the data that was assigned to it. (In C)

Question

I am new to this site, so I greatly apologize if I do anything wrong with this first post. The way I've written my code (including codes made for reusability), I must use char* arrays. I am converting the passed in char* into all lower-case letters. The problem I am facing deals with my freeing of newString2. Because i'm assigning it to my newString, freeing it would definitely lose the information I assigned to newString, thus losing the data assigned to convertedKey. I am in a hole as to finding out how to successfully free allocated memory, but also return the data of the converted string through my function argument. Is that possible? The following is my current function for converting to lower-case. I am also very new to strdup, so I'm sure I'm not giving it any justice.

void convertKeyCasing (ListElementPtr key, ListElement *convertedKey)
{
    ListElementPtr newString = (ListElementPtr) malloc (sizeof (ListElement));
    int i = 0;
    char ch;
    char *string = key->data.key.key;

    char *newString1 = strdup (string);

    while (isalpha (key->data.key.key[i]) != false)
    {
        ch = tolower(key->data.key.key[i]);

        newString1[i] = ch;

        ++i;
    }

    char* newString2 = strdup (newString1);

    newString->data.key.key = newString2;

    *convertedKey = *newString;

    key = convertedKey;

    free(newString2);
    free (newString1);
    free (newString);
}

My structs in list include:

typedef struct KEY_DATATYPE
{
    char* key;
    int length;
}KEY_DATATYPE;

typedef struct DATATYPE
{
    Q_DATATYPE data;

    KEY_DATATYPE key;

    // maintained so that it still works with list
  union
  {
    char charValue;
    unsigned int intValue;
    float floatValue;
  };
  unsigned short whichOne;
} DATATYPE;

typedef struct ListElement* ListElementPtr;
typedef struct ListElement
{
    DATATYPE data;
    ListElementPtr psNext;
    ListElementPtr psPrev;
} ListElement;

And in my driver, I'm creating a ListElementPtr and assigning it a key value, for example,

ListElementPtr listPtr;
listPtr->data.key.key = "Hello";

Thank you again for the help!

Answer 1

There are simply lots and lots of errors in your code. You leak memory, and setup pointers to memory that you then free. Rather than trying to explain all the errors, I'll attempt to show you how to write the code most simply.

Essentially, from what I can tell, you simply want to convert everything in key->data.key.key to lower case. There is as such no need for any dynamic allocation. You can perform the modifications in-place like this:

void convertKeyToLower(ListElementPtr key)
{
    char *ch = key->data.key.key;
    while (*ch != 0)
    {
        *ch = tolower(*ch);
        ch++;
    }
}

In fact this function is still badly designed. It is mixing together two distinct aspects: list elements, and string handling. I would split the string handling into a separate function:

void convertStringToLower(char *str)
{
    while (*str != 0)
    {
        *str = tolower(*str);
        str++;
    }
}

Or perhaps like this:

void convertStringToLower(char *str)
{
    for(int i = 0; str[i]; i++)
        str[i] = tolower(str[i]);
}

Once you have this at your disposal you can simply write:

convertStringToLower(element->->data.key.key);

And any time that you need to convert a string, in-place, to lower-case, you have the function ready and available.

---

From your update to the question, it is now clear why you are getting the segmentation fault that you report in the comments. You wrote:

listPtr->data.key.key = "Hello";

This makes listPtr->data.key.key point to a string literal. And string literals are not modifiable. Attempting to do so invokes undefined behaviour (UB) and a typical manifestation of that UB is a segmentation fault. That happens because compilers typically store string literals in read-only memory.

You need to decide whether or not your elements are going to store literals or modifiable strings. You cannot really hope to mix the two. If you attempt to mix the two then you won't be able to determine how to deallocate your elements. That's because you need to free a modifiable string, but you must not attempt to free a literal.

So your only reasonable option is to use modifiable strings always, and always free them when you free your elements. So replace the line of code above with:

listPtr->data.key.key = strdup("Hello");