2

How should I change the variable BinaryNumber, so the function will not give me a warning message. I understand that I can't return a address of a local variable cause of the memory. Should I use malloc or what do you think how should I change the variable BinaryNumber so I can return it?

char *Functoin_chang_string_ToBinary(char *Line) {
    char *space = " ", *point = ",";
    char BinaryNumber[Array_Size] = { "000000000000000" };
    char *word = strtok(Line,"\"");
    int num = 0, flag = 2, j = 11;
    for (int i = 0; i < strlen(Line) - 1; i++) {
        if (word[i] == '"') {
            flag--;
            i++;
        }
        num = word[i];
        j = 14;
        while (num != 0) {
            BinaryNumber[j--] += (num % 2);
            num /= 2;
        }
        printf("%s\n", BinaryNumber);
        Fill_Struct_Binary_Machine_Code("", BinaryNumber);
        strcpy(BinaryNumber, "000000000000000");
    }
    printf("%s\n", BinaryNumber);
    Fill_Struct_Binary_Machine_Code("", BinaryNumber);

    return BinaryNumber;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
Dando
  • 39
  • 3
  • 1
    Yes, I think is ok using malloc – Eduardo Pascual Aseff Mar 29 '20 at 23:25
  • 3
    I would say make the caller provide the buffer (as strtol does as an example). – SoronelHaetir Mar 29 '20 at 23:31
  • 3
    I agree with Soronel. I've found it to be much cleaner to allow the caller to supply the buffer. That way, they can pass memory local to their scope if they want which can be free'd automatically. If you return a `malloc`'d pointer, you're forcing your caller to manually manage the memory. That's assuming it's feasible for the user to know how big of a buffer to supply. – Carcigenicate Mar 29 '20 at 23:33

3 Answers3

2

Although you can use malloc() to allocate a buffer for the BinaryNumber string (which will, of course, need to be released by the caller using free()), a much simpler method would be to use the strdup() function.

This will allocate the exact amount of memory required to duplicate the string given as its argument (including the nul terminator) and copy the actual string data in one call.

Thus, you could just change your function's return statement to this:

    return strdup(BinaryNumber);

Of course, the caller will still need to call free() on the returned data when it's done with it (strdup allocates the memory in a manner compatible with the malloc function).

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
  • Neither `strdup` nor `_strdup` is defined by the ISO C standard, as far as I can see. – Nate Eldredge Mar 30 '20 at 02:12
  • @NateEldredge Well, I'm only going by what `clang-cl` tells me (and `MSVC` says something very similar): *"warning : 'strdup' is deprecated: The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _strdup. See online help for details. [-Wdeprecated-declarations]"* So, it's not so much the **function** that's ISO C, but the **naming convention**? – Adrian Mole Mar 30 '20 at 02:46
  • @NateEldredge: `strdup()` is normalized in POSIX and supported on all modern systems. It is easy to implement on legacy systems that do not conform to POSIX. After many years of procrastination, `strdup()` is finally making its way into the C Standard, and will be included in the next version. Here is the latest draft as of March 30, 2020: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2479.pdf – chqrlie Mar 30 '20 at 12:07
  • @AdrianMole: the function will ne included along with `strndup()` in the next version of the C Standard, with the name `strdup()`. I am sure Microsoft lobbied to have the `_` prefix which they invented in the 1980's. The MSVC warning is fake news: the POSIX name is `strdup` as can be verified here: https://pubs.opengroup.org/onlinepubs/9699919799/functions/strdup.html . See this [answer](https://stackoverflow.com/a/13575979/4593267) for details – chqrlie Mar 30 '20 at 12:14
  • @AdrianMole: More precisely, Microsoft unilaterally decided to deprecate the POSIX names in favor of their prefixed counterparts for those POSIX functions it supports. A typical example of the [Embrace, extend, and extinguish](https://en.wikipedia.org/wiki/Embrace,_extend,_and_extinguish) strategy. – chqrlie Mar 30 '20 at 12:27
  • @chqrlieforyellowblockquotes Thanks for the clarification! I have removed the reference to the *Microsoft* ISO C Standard! (Working on the principle that anyone who should come across the warning will know how to handle it.) – Adrian Mole Mar 30 '20 at 13:16
1

Change your function signature to accept a target to save it to:

char *Function_chang_string_ToBinary_r(char *Line, char *out, size_t out_size )

Whenever you do this it is far safer to provide a maximum output size as not to overrun the target. You would also use size sensitive copies for copying to the target limiting copies to the smaller of the target area our your internal working area.

Look at the strtok() vs strtok_r() function for a model for a function that switched to this model to be thread safe.

Yes, malloc() would work but I tend to consider malloc() calls within a function a bad idea, expecialy for short strings. [1] it leaves callers open to memory leaks if they forget to free the memory you allocated and [2] if the function is called frequently malloc() can have a high overhead. Passing in a NULL pointer could make the function call malloc() anyway to return the new address. This way any memory leak or performance bugs would then be on the caller.

Gilbert
  • 3,740
  • 17
  • 19
1

Yes, you can return a pointer to memory allocated by malloc.

Alternatively, you could change the function prototype of Functoin_chang_string_ToBinary to the following:

void Functoin_chang_string_ToBinary(char *Line, char *BinaryNumber );

That way, the calling function can allocate the memory as a local array and pass a pointer to that array to the function Functoin_chang_string_ToBinary, for example like this:

    BinaryNumber[Array_Size];

    Functoin_chang_string_ToBinary( Line, BinaryNumber );

However, when passing pointers to memory buffers like this, it is also important to make sure that the called function does not write past the boundary of the buffer. For this reason, it would be better if the called function knew the size of the buffer it is being passed. Therefore, you may want to also pass the size of the buffer to the function, by changing the function prototype to the following:

void Functoin_chang_string_ToBinary(char *Line, char *BinaryNumber, int BinaryNumberSize )

The code of the calling function would then be changed to the following:

    BinaryNumber[Array_Size];

    Functoin_chang_string_ToBinary( Line, BinaryNumber, ArraySize );

That way, the called function can determine how large the memory buffer is, by accessing its third parameter.

Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39