1

I found myself using function parameters for both input and output and was wondering if what I'm doing is going to bite me later.

In this example buffer_len is such a parameter. It is used by the foo to determine the size of the buffer and tell the caller, main, how much of the buffer has been used up.

#define MAX_BUFFER_LENGTH 16
char buffer[MAX_BUFFER_LENGTH] = {0};

void main(void)
{
    uint32_t buffer_len = MAX_BUFFER_LENGTH;

    printf("BEFORE: Max buffer length = %u", buffer_len);

    foo(buffer, &buffer_len);

    printf("BEFORE: Buffer length used = %u", buffer_len);
}

void foo(char *buffer, uint32_t *buffer_len)
{
    /* Remember max buffer length */
    uint32_t buffer_len_max  = *buffer_len;
    uint32_t buffer_len_left =  buffer_len_max;

    /* Add things to the buffer, decreasing the buffer_len_left 
       in the process */
    ...

    /* Return the length of the buffer used up to the caller */
    *buffer_len = buffer_len_max - buffer_len_left;
}

Is this an OK thing to do?

EDIT:

Thank you for your responses, but I'd prefer to keep the return value of foo for the actual function result (which makes sense with larger functions). Would something like this be more pain-free in the long run?

typedef struct
{
    char *data_ptr;
    uint32_t length_used;
    uint32_t length_max;
} buffer_t;

#define ACTUAL_BUFFER_LENGTH 16
char actual_buffer[ACTUAL_BUFFER_LENGTH] = {0};

void main(void)
{
    buffer_t my_buffer = { .data_ptr    = &actual_buffer[0],
                           .length_used = 0,
                           .length_max  = ACTUAL_BUFFER_LENGTH };
}
TylerH
  • 20,799
  • 66
  • 75
  • 101
andrey
  • 1,515
  • 5
  • 23
  • 39
  • 1
    It's "OK" as long as it is documented, but really not the preferred way of operating. Why not have the function return the length used, and leave the buffer length parameter as a regular `uint32_t` (or perhaps `size_t`, so you can pass `sizeof(buffer)` to the function)? – Jonathan Leffler Aug 18 '15 at 15:36
  • 2
    It's syntactically OK, but personally I much prefer return codes. If that is not possible, I would want a *dedicated* output parameter, *especially* if it's pass-by-reference. (I might not pay attention and continue under the assumption that my `buffer_len` still holds the original value.) – DevSolar Aug 18 '15 at 15:36
  • 1
    As long as you have not used up your return value, I prefer to receive the result rather than having to defining a variable every time I use the function (or maybe I want to input an expression). – BeyelerStudios Aug 18 '15 at 15:36
  • 3
    Don't show a `void` function if your real function is going to return a value. Doing so completely alters the answers. If you need to return more than one value, then an in-out parameter is OK (even necessary — `getchar()` is a counter-example), though a pure in parameter and a separate pure out parameter might be better. Using a structure is OK too. Note, however, that `void main(void)` is not the standard way to write `main()` — see [What should `main()` return in C and C++?](http://stackoverflow.com/questions/204476/what-should-main-return-in-c-and-c/18721336#18721336) for the full story. – Jonathan Leffler Aug 18 '15 at 15:56

2 Answers2

4

For the original version of the question where the called function doesn't return a value, you got three similar answers, all roughly saying "Yes, but…":

Jonathan Leffler said:

It's "OK" as long as it is documented, but really not the preferred way of operating. Why not have the function return the length used, and leave the buffer length parameter as a regular uint32_t (or perhaps size_t, so you can pass sizeof(buffer) to the function)?

DevSolar said:

It's syntactically OK, but personally I much prefer return codes. If that is not possible, I would want a dedicated output parameter, especially if it's pass-by-reference. (I might not pay attention and continue under the assumption that my buffer_len still holds the original value.)

BeyelerStudios said:

As long as you have not used up your return value, I prefer to receive the result rather than having to defining a variable every time I use the function (or maybe I want to input an expression).

The unanimity is remarkable.

The question was then updated to indicate that instead of returning void, the function's return value would be used for another purpose. This completely changes the assessment.

Don't show a void function if your real function is going to return a value. Doing so completely alters the answers. If you need to return more than one value, then an in-out parameter is OK (even necessary — getchar() is a counter-example), though a pure in parameter and a separate pure out parameter might be better. Using a structure is OK too.

Perhaps I should explain the 'counter-example' a bit. The getchar() function returns a value that indicates failure or a char value. This leads to many pitfalls for beginners (because getchar() returns an int, not a char as its name suggests). It would be better, in some ways, if the function was:

bool get_char(char *c);

returning true if it reads a character and false if it fails, and assigning the character value to c. It could be used like:

char c;
while (get_char(&c))
    …use character just read…

This is a case where the function needs to return two values.

Harking back to the suggested revised code in the question, the code using the structure.

That is not a bad idea at all; it is often sensible to package up a set of values into a structure. It would make a lot of sense if the called function has to compute some value that it will return and yet it also modifies the buffer array and needs to report on the number of entries in it (as well as knowing how much space there is to be used). Here, keeping the 'space available' separate from the 'space used' is definitely preferable; it will be easier to see what's going on than having the 'in-out' parameter which informs the function how much space is available on entry and reports back how much space was used on exit. Even if it reported how much space was still available on exit, it would be harder to use.

Which gets back to the original diagnosis: yes, the in-out parameter is technically legal, and can be made to work, but isn't as easy to use as separate values.


Side note: void main(void) is not the standard way to write main() — see What should main() return in C and C++? for the full story.

Community
  • 1
  • 1
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
1

There's nothing wrong with using the same buffer for input and output, but it probably limits the functions utility elsewhere. For example, what if you want to use it with two different values? (for some reason as a user of the function I need the original preserved). In the example you provide there's no harm in taking two parameters in the function and then just passing the same pointer in twice. Then you've wrapped up both uses and it probably simplifies the function code.

For more complex data types like arrays, as well as the same problem above, you'll need to make sure your function doesn't need a larger output, or if it shrinks the buffer that you memset( 0.. ) the difference and so on.

So for those headaches I do tend to avoid as a pattern, but as I say nothing particularly wrong.

James
  • 224
  • 2
  • 13