15

Upon code review/clang-tidy runs, I came across a function with a signature like this:

void appendFoo(const char * fmt, va_list& rVaList);

I have never seen this before. Afaik, you can pass va_list by value (and maybe by pointer?).

This brings me to my first question: Is passing va_list by reference legal and does it work as expected?

Anyway, appendFoo() calls vsprintf() in its definition and clang-tidy gave the following warning: Function 'vsprintf' is called with an uninitialized va_list argument [clang-analyzer-valist.Uninitialized]. The definition of appendFoo() look basically like this:

void appendFoo(const char * fmt, va_list& rVaList) {
  // retracted: allocate buffer

  vsprintf(buffer, fmt, rVaList);

  // errorhandling
  // de-allocate buffer
}

(Yes, the return value of vsprintf is ignored and errors are "handled" another way. I'm fixing it ...)

In particular, no va_copy, va_start, etc. are called.

Removing the reference and passing va_list by value, i.e. changing the signature to void appendFoo(const char * fmt, va_list rVaList);, eliminates the clang-tidy warning.

This brings me to my second question: Is the warning produced by clang-tidy if va_list is passed by reference a false-positive or is there is actually a problem in passing va_list as reference?

PS: This question is not a duplicate of varargs(va_list va_start) doesn't work with pass-by-reference parameter. The OP of that question passes an argument by reference to a function taking a va_list. I'm asking about va_list itself being passed as a reference. Oh, well.

Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
user3284229
  • 151
  • 3
  • 4
    In my opinion va_list should not be used in C++ at all (it is a "C" relic that is not typesafe, prone to errors/safety issues etc.). You should use templates with parameter packs : https://en.cppreference.com/w/cpp/language/parameter_pack, and forget all about va_list – Pepijn Kramer Feb 13 '23 at 14:06
  • I think this is illegal. Because `va_list` definition is *unspecified* : https://en.cppreference.com/w/c/variadic/va_list I guess an implementation is free to use something incompatible with references (although I cannot find an example). – prapin Feb 13 '23 at 14:14
  • 3
    Unlike @PepijnKramer, I don't think we should avoid `va_list` at C++. But use them very carefully. On our C++ code base, string formatting functions are variadic template functions building up a type string, that is passed by to a private variadic C function. Inspired from here (but heavily enhanced afterwards): https://gist.github.com/deplinenoise/6297411 – prapin Feb 13 '23 at 14:24
  • Nice question! Also take a look at the [tour](https://stackoverflow.com/tour) and the [How to Ask](https://stackoverflow.com/help/how-to-ask) page. – justANewb stands with Ukraine Feb 13 '23 at 14:25
  • @prapin are there types that are incompatible with references? – Giovanni Cerretani Feb 13 '23 at 14:30
  • That seems like either a false positive (since passing a pointer is valid, I would expect the same from a reference) or a problem with how you're calling `appendFoo`. – molbdnilo Feb 13 '23 at 14:30
  • @GiovanniCerretani: if `va_list` is implemented as a macro, chances are that using it with a reference could lead to unexpected results... – Serge Ballesta Feb 13 '23 at 14:33
  • 2
    `va_list` cannot be implemented as a macro, it must be a complete object type. Anyway, I asked a similar question, you may find some details in the comments of this answer: https://stackoverflow.com/a/60041534/3287591 In short: since it is unclear if it is legal to use references to `va_list`, use pointers to `va_list` that are allowed as per C standard. – Giovanni Cerretani Feb 13 '23 at 14:37
  • 2
    The question is not a duplicate. It is about passing `va_list` itself as reference, the other question talks about the second argument passed to `va_start`. – Giovanni Cerretani Feb 13 '23 at 14:39
  • 1
    @GiovanniCerretani I have re-opened it – user253751 Feb 13 '23 at 14:47
  • There is no gain in passing va_list by reference. It's just a pointer. – n. m. could be an AI Feb 13 '23 at 15:13
  • 1
    I see no reason to believe that it wouldn't work. As was mentioned above, `va_list` must be a complete object type, so references to it can't behave strangely. It can't be something like `void` (where forming a reference would be ill-formed). – Brian Bi Feb 13 '23 at 16:08
  • "In particular, no va_copy, va_start, etc. are called" - Not calling `va_start` and `va_end` is certainly a bug - it's [UB](https://en.cppreference.com/w/cpp/language/ub) to not do so. – Jesper Juhl Sep 01 '23 at 10:33

1 Answers1

2

Yes, it is allowed. Firstly, [cstdarg.syn] states:

The contents of the header are the same as the C standard library header <stdarg.h>, with the following changes:

  • [...]

None of the restrictions here disallow forming a reference to a std::va_list.

Relevant wording in the C standard

The answer lies in the C17 standard, 7.16 Variable arguments <stdarg.h>, p3:

The type declared is

va_list

which is a complete object type suitable for holding information needed by the macros va_start, va_arg, va_end, and va_copy. If access to the varying arguments is desired, the called function shall declare an object (generally referred to as ap in this subclause) having type va_list. The object ap may be passed as an argument to another function; if that function invokes the va_arg macro with parameter ap, the value of ap in the calling function is indeterminate and shall be passed to the va_end macro prior to any further reference to ap.257)

257) It is permitted to create a pointer to a va_list and pass that pointer to another function, in which case the original function may make further use of the original list after the other function returns.

Essentially, passing a std::va_list by pointer allows the callee to use the std::va_list as if we had used it in our own function. Since std::va_list is a complete object type, binding a reference to it is possible.

It's also worth noting that pointers and references are identical in the ABI, so both result in passing a memory address to a function and behave more or less the same.

See also: Pass va_list or pointer to va_list?

Passing va_list by reference is pointless in your case

However, the use of a reference to std::va_list is pointless in your example:

void appendFoo(const char * fmt, va_list& rVaList) {
    vsprintf(buffer, fmt, rVaList);
}

The issue is that std::vsprintf accepts a std::va_list by value, so when it calls va_arg internally, the std::va_list in the caller of appendFoo becomes indeterminate.

You could have just as well passed std::va_list by value, which would have been no worse.

About clang-tidy

The rule you're presumably talking about is cppcoreguidelines-pro-type-vararg which flags all uses of C variadics.

You're not directly calling va_arg, so it's not entirely clear if your use should be flagged. Overall, it's not surprising though, and you probably want to disable this check if you're working with C variadics.

When does it make sense to pass va_list by reference or pointer?

It makes sense if you want to keep using the list in the caller of the function:

std::array<int, 3> get_triple(std::va_list& args_ref) {
    return {
        va_arg(args_ref, double), va_arg(args_ref, double), va_arg(args_ref, double)
    };
}

// ...
std::va_list args;
va_start(args, /* ... */);
auto [x, y, z] = get_triple(args);
auto [u, v, w] = get_triple(args);
// ...

If we hadn't passed args_ref by reference, then the use of va_arg in get_triple would have made args indeterminate.

Jan Schultke
  • 17,446
  • 6
  • 47
  • 96