0

Is it fine to use char pointer(char *retStatus) in below way? Like assigning/rewriting values whenever required without allocating memory? I tested it and it is working fine but would like to know is this a good approach to assign error messages to char * and then copy/concat to other static or allocated memory pointer.

void fn(char *status, size_t maxLen)
{
    char *retStatus = NULL;
    ...
    ...

    if(failure)
    {
        retStatus = "error1";   
        if((strlen(retStatus) + strlen(status)) < maxLen)
        {
            strcat_s(status, maxLen, retStatus);
        }
    }

    ...
    ...

    if(failure)
    {
        retStatus = "error2";   
        if((strlen(retStatus) + strlen(status)) < maxLen)
        {
            strcat_s(status, maxLen, retStatus);
        }
    }
}

int main()
{
    char status[10] = { 0 };
    size_t statusMaxLen = sizeof(status) / sizeof(status[0]);
    fn(status, statusMaxLen);
    return 0;
}
Arun
  • 2,247
  • 3
  • 28
  • 51
  • 1
    You should probably use `strncat` and the like. – Fiddling Bits Feb 25 '20 at 16:36
  • 1
    @Arun There is no problem. – Vlad from Moscow Feb 25 '20 at 17:02
  • 1
    Mostly okay. I do notice that you used generic "error1" and "error2" so I assume these values are just placeholders for something else that will be decided at a later time. Because of that, status[] only being 9 working characters seems a little small. It all, of course, depends on how much you know about the values that will be selected and who will select them. True that strcat_s protects you, but do you want to keep coming back here to fiddle with the size of this? – Frank Merrow Feb 25 '20 at 17:14
  • yes. the error messges are placeholders and my static array size is large. – Arun Feb 25 '20 at 17:19
  • What “assigning/rewriting values whenever required without allocating memory” are you concerned with? Memory is reserved for `status` by its definition in `main`, and, although it is small, the code takes care not to use more than is reserved. The assignments such as `retStatus = "error1";` do not write characters into memory; they just set a pointer to point to existing strings. (String literals represent defined arrays that exist throughout program execution.) – Eric Postpischil Feb 25 '20 at 17:33

2 Answers2

0

Is it fine to use char pointer(char *retStatus) in below way? Like assigning/rewriting values whenever required without allocating memory?

A string literal represents an array of char with static storage duration, with the (rather substantial) restriction that any attempt to modify the array contents produces undefined behavior. You may use string literals in any way that you may use any other char array, subject to the restriction that you do not attempt to modify them.

With that said, it is better form to avoid assigning string literals to variables of type char *, or passing them as function arguments corresponding to parameters of that type. Instead, limit yourself to pointers of type const char *, which convey the relevant restriction explicitly.

I tested it and it is working fine but would like to know is this a good approach to assign error messages to char * and then copy/concat to other static or allocated memory pointer.

The particular combination of assignment followed by non-mutating access is allowed, and will work reliably, but again, it would be better to use a variable of type const char * instead of a variable of type char *. Do note, however, that it is possible to get yourself into trouble that way if you're not careful. For example, sizeof("error1") is unlikely to be equal to (retStatus = "error1", sizeof(retStatus)).

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
-1

This is a valid and indeed a clever way of using pointers. The approach works fine for the given example.

The only possible issue with the approach, in the context of big and long running programs, is with the intended lifetime of the variable.

If memory is allocated explicitly using malloc, then it can also be deleted whenever the variable is no longer required. The explicit memory management will help conserve memory allocation and improve the overall performance.

In the current approach, the memory allocated to the variable will persist throughout the running time of the program.

If it is desirable to have the variable persist throughout the program runtime, then the followed approach is perfect.

If conservation of memory is a crucial requirement, then using malloc and free is a recommended approach.

Gopinath
  • 4,066
  • 1
  • 14
  • 16
  • 3
    C has `free`, not `delete`. – Eric Postpischil Feb 25 '20 at 17:29
  • 2
    It is unclear what “memory allocated variable“ that will persist through program execution this answer is referring to. The string literals `"error1"` and `"error2"`. None of the other objects in this program have static storage duration. The `status` object defined in `main` has automatic storage duration, although it does persist through most of program execution since it is defined in `main`. But putting a `malloc` call at the start of `main` and a `free` at the end would not improve this. – Eric Postpischil Feb 25 '20 at 17:29
  • Hi Eric, the use of malloc and free is being recommended as a general best practice that can be used in bigger programs that run for long durations. As pointed by you, the use of malloc and free in a small program that runs for only a few milliseconds will not provide any tangible benefits. Benefits can be definitely found in long running programs. – Gopinath Feb 25 '20 at 18:16
  • @Gopinath, the question is what memory are you supposing would be dynamically allocated and freed? You raise dynamic allocation as an issue with the OP's code, so I presume you can tie your comments back to that. The only things I can think of that you might be talking about is replacing the string literals with allocated memory, but either you have a misunderstanding about what's going on here or else you need to think through how one might actually implement the dynamic allocation you have in mind, and what the effect actually would be. – John Bollinger Feb 25 '20 at 18:21
  • Hi John, I already answered that the approach of not using malloc is perfectly fine for the given example code. The recommendation on using malloc is applicable only for big and long running programs where dynamic memory management is a crucial requirement. – Gopinath Feb 25 '20 at 18:51
  • @Gopinath, even in a long-running program with tight memory constraints, I am not seeing how you suppose that dynamic allocation would make sense for a function such as the OP's. Regardless of how *needful* it might be in the OP's example case, I am asking you to explain how you imagine dynamic allocation being used there to achieve any reduction in memory usage. Or are you saying that that bit of your answer is a total *non sequitur*? – John Bollinger Feb 25 '20 at 19:44
  • @Gopinath: My comment said nothing about how long the program takes. My comment points out your answer is not clear about **what** object it is talking about allocating. The buffer named `status`? The string literals? Something else? – Eric Postpischil Feb 25 '20 at 19:45