2

I have function taking a variable and NULL terminated list of pointer arguments using the elipsis. I know about variable length template argument lists. It is about legacy code. Will the following two calls lead to undefined behaviour because the terminator is interpreted as Serializable* by va_arg? What are the differences between the two calls?

void serialize(Serializable* first, ...) {
    va_list vl;

    va_start(vl, first);

    while(1)
    {
        Serializable* arg = va_arg(vl, Serializable*);

        if(arg == NULL)
            break;

        /* serialize arg here */
    }
}

serialize(obj1, obj2, obj3, NULL);
serialize(obj1, obj2, obj3, nullptr);
Martin Fehrs
  • 792
  • 4
  • 13
  • Is it possible to corret the Typo in the title? – Martin Fehrs Feb 04 '19 at 11:37
  • Yes, just [edit] your post. – StoryTeller - Unslander Monica Feb 04 '19 at 11:37
  • Why not use variadic templates instead of this very bad C-style code? – Matthieu Brucher Feb 04 '19 at 11:39
  • 2
    @MatthieuBrucher *"I know about variable length template argument lists. It is about legacy code."* - OP – HolyBlackCat Feb 04 '19 at 11:39
  • Even if it's legacy code, you can still change it to use better typed code. Except if you are stuck with an old compiler. – Matthieu Brucher Feb 04 '19 at 11:41
  • It is not real legacy code. It is for a presentation. I want to find out, if there are any risks switching to from NULL to nullptr for existing code. – Martin Fehrs Feb 04 '19 at 11:42
  • @MatthieuBrucher No. Changing code in such a fundamental way has a cost. You don't just rewrite a legacy application for the sake of being "more modern". Or, if you do, I fire you ;) – Lightness Races in Orbit Feb 04 '19 at 11:43
  • 1
    @LightnessRacesinOrbit Well, it depends, I don't write such things for the sake of being modern. I would write tests first, but if you would not consider changing the code while having tests, then I would probably fire you. – Matthieu Brucher Feb 04 '19 at 11:53
  • @MatthieuBrucher It's called "legacy" for a reason. Unless you are making the application non-legacy (which is a whole other discussion), changing the structure of such an old application purely for style reasons is frankly misconductful. Sure, you have tests, but you can't guarantee not introducing bugs, and _what is the benefit against this risk?_ I understand that the new code may be more easy to maintain etc, but then those are valid reasons to _de-legacy_ the code and so again we're back to the discussion not being about changing a legacy app _per se_. – Lightness Races in Orbit Feb 04 '19 at 11:56
  • Obviously OP was tasked with some refactorings which can be defined as stylish, so obviously this change can also be on the table. – Matthieu Brucher Feb 04 '19 at 11:58
  • @MatthieuBrucher Sorry, where was that mentioned? I must have missed a comment. – Lightness Races in Orbit Feb 04 '19 at 12:04

1 Answers1

2

No, I don't think so.

Quoting cppreference.com on va_arg:

If the type of the next argument in ap (after promotions) is not compatible with T, the behavior is undefined, unless:

  • one type is a signed integer type, the other type is the corresponding unsigned integer type, and the value is representable in both types; or
  • one type is pointer to void and the other is a pointer to a character type (char, signed char, or unsigned char).

(This very closely matches the actual C11 wording; remember, va_arg is defined by C, not C++.)

Now, C11's definition of "compatible types" is summed up by another cppreference, which teaches us that for your NULL to have a type compatible with Serializable*, the pointee-type of NULL would have to be compatible with Serializable.

Now, NULL has an implementation-defined type so you can't know what it is, but it's certainly not going to be one that's compatible with Serializable, unless that is simply a type alias for void or int and you get lucky.

With the nullptr you get void*, but then again see above.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • 1
    It's about "compatibility" in the C sense, me thinks. And if [I read correctly](https://en.cppreference.com/w/c/language/type), `void*` (which is [what `nullptr_t` is converted to](https://timsong-cpp.github.io/cppwp/n4659/expr.call#9) when passed as an extra ellipsis argument) is not compatible with `Serializable*`. – StoryTeller - Unslander Monica Feb 04 '19 at 11:48
  • Aren't promotions implicit conversions? – Martin Fehrs Feb 04 '19 at 11:49
  • 1
    We can make an analogy with `printf("%p", x)`. GCC allows `x` to be `void *` or `nullptr_t`, but issues a warning for any other pointer type and `NULL`. But I agree that the warning could be broken... – HolyBlackCat Feb 04 '19 at 11:51
  • @StoryTeller Agreed. Was researching :) (a bit late :P) – Lightness Races in Orbit Feb 04 '19 at 11:51
  • 1
    @MartinFehrs Promotions are a limited subset of all implicit conversions. That is, you can classify them as implicit conversions, but that doesn't mean "promotion" entails _any possible implicit conversion_. Promotions are basically just arsing about making `int`s bigger – Lightness Races in Orbit Feb 04 '19 at 11:52
  • So both versions lead to undefined behaviour? Is this a real world problem? I've seen code written like this. – Martin Fehrs Feb 04 '19 at 12:07
  • @MartinFehrs Looks like it! People abuse `NULL` all the time tbf – Lightness Races in Orbit Feb 04 '19 at 12:29