1

I wrote a Helper class with c functions for an iOS Library with the following pattern. There are 2 wrapping (variadic) functions, which finally call the same function, with slightly different parameter. Idea is to have "default" properties being set.

__attribute__((overloadable)) void func1(NSString* _Nonnull format, ...);
__attribute__((overloadable)) void func1(int param1, NSString* _Nonnull format, ...);

Both will then call the following function:

void prefixAndArguments(int param1, NSString* _Nonnull format, va_list arguments);

Implementation as followed:

__attribute__((overloadable)) void func1(NSString* _Nonnull format, ...)
{
    va_list argList;
    va_start(argList, format);
    prefixAndArguments(0, format, argList);
    va_end(argList);
}

__attribute__((overloadable)) void func1(int param1, NSString* _Nonnull format, ...)
{
    va_list argList;
    va_start(argList, format);
    prefixAndArguments(param1, format, argList);
    va_end(argList);
}


void prefixAndArguments(NMXLogLevelType logLevel, NSString* _Nullable logPrefix, __strong NSString* _Nonnull format, va_list arguments)
{
    // Evaluate input parameters
    if (format != nil && [format isKindOfClass:[NSString class]])
    {
        // Get a reference to the arguments that follow the format parameter
        va_list argList;
        va_copy(argList, arguments);

        int argCount = 0;
        NSLog(@"%d",argCount);
        while (va_arg(argList, NSObject *))
        {
            argCount += 1;
        }
        NSLog(@"%d",argCount);
        va_end(argList);

        NSMutableString *s;
        if (numSpecifiers > argCount)
        {
            // Perform format string argument substitution, reinstate %% escapes, then print
            NSString *debugOutput = [[NSString alloc] initWithFormat:@"Error occured when logging: amount of arguments does not for to the defined format. Callstack:\n%@\n", [NSThread callStackSymbols]];
            printf("%s\n", [debugOutput UTF8String]);
            s = [[NSMutableString alloc] initWithString:format];
        }
        else
        {
            // Perform format string argument substitution, reinstate %% escapes, then print
            va_copy(argList, arguments);

            // This is were the EXC_BAD_ACCESS will occur!
            // Error: Thread 1: EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
            s = [[NSMutableString alloc] initWithFormat:format arguments:argList];
            [s replaceOccurrencesOfString:@"%%"
                               withString:@"%%%%"
                                  options:0
                                    range:NSMakeRange(0, [s length])];
            NSLog(@"%@",s);
            va_end(argList);
        }
    ...
}

My Unit Tests for the function look the following (order is important).

// .. some previous cases, I commented out
XCTAssertNoThrow(NMXLog(@"Simple string output"));
XCTAssertNoThrow(NMXLog(@"2 Placeholders. 0 Vars %@ --- %@"));

The crash happens when I want to use the arguments and the format (making format strong did not solve the problem, and does not seem being part of the problem, see below):

s = [[NSMutableString alloc] initWithFormat:format arguments:argList];

Here is the Log:

xctest[28082:1424378] 0
xctest[28082:1424378] --> 1
xctest[28082:1424378] Simple string output
xctest[28082:1424378] 0
xctest[28082:1424378] --> 4

Of course we won't see the desired string "2 Placeholders. 0 Vars %@ --- %@" as the crash happened before.

So, the question is now: Why is the amount of arguments now being 4 instead of 0? As none being passed in the second call, are the arguments being collected when the function is being called immediately again?

So, I started to call the function "again" to make sure the argument's list is being cleared, although va_end was being called:

__attribute__((overloadable)) void func1(NSString* _Nonnull format, ...)
{
    va_list argList;
    va_start(argList, format);
    prefixAndArguments(none, nil, format, argList);
    va_end(argList);
    NSString *obj = nil;
    prefixAndArguments(none, nil, obj, nil);
}

This does work now like a charm (argument's list is being cleared and the desired output is being received):

xctest[28411:1453508] 0
xctest[28411:1453508] --> 1
xctest[28411:1453508] Simple string output
xctest[28411:1453508] 0
xctest[28411:1453508] --> 1
Error occured when logging: amount of arguments does not for to the defined format. Callstack: ....
xctest[28411:1453508] 2 Placeholders. 0 Vars %@ --- %@

Here is finally my question:

What is the reason for this behavior and how can I avoid it? Is there a better way to solve the issue than "stupidly" calling the function a second time with "no" arguments to clear the them? P.s. I tried not to use macros, because I consider them as more error prone than c functions. See this thread: Macro vs Function in C

Lepidopteron
  • 6,056
  • 5
  • 41
  • 53
  • 1
    Ask yourself this: how does `va_arg` know when to stop? – jscs Oct 17 '17 at 13:23
  • This seems to be a heck of a lot of trouble to go through to implement an optional first argument. How about instead just ... not. – John Bollinger Oct 17 '17 at 13:55
  • @JohnBollinger thank you for your inout John. For reasons of simplicity I just mentioned two functions with one optional parameter. Of course, there are more. – Lepidopteron Oct 17 '17 at 13:58
  • Is the `NMXLogWithPrefixAndArguments()` function you've presented supposed to be the same thing as the `prefixAndArguments()` function you talk about and your code calls? – John Bollinger Oct 17 '17 at 14:02
  • Thank you for your hint, I missed this when preparing this post, this one is supposed to also be `prefixAndArguments`(I have updated the question) – Lepidopteron Oct 17 '17 at 14:03

1 Answers1

2

You appear to have some misconceptions about variadic functions, exemplified by this approach to counting the variable arguments:

        while (va_arg(argList, NSObject *))
        {
            argCount += 1;
        }

That code assumes that the variable arguments have at least one member, that all of them are of type NSObject *, and that the list will be terminated by a null pointer of that type. None of those is guaranteed by the system, and if those assumptions are not satisfied then the behavior of one or more va_arg() invocations will be undefined.

In practice, you can probably get away with actual arguments that are pointers of other types (though formally, the behavior will still be undefined in that case). If the arguments may have non-pointer types, however, then that approach to counting them is completely broken. More importantly, your test cases appear to assume that the system will provide a trailing NULL argument, but that is in no way guaranteed.

If your function relies on the end of the variable argument list being signaled by a NULL argument, then it is relying on the caller to provide one. It is very likely the absence of null termination in your argument lists that gives rise to the behavior you are asking about.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
  • Thank you John for clarifying that. The function shall be used in a library, thus I want to ensure users do not crash their code, if they do not append the expected sentinel value. Can one append a such to the argument's list before calling the wrapped function `prefixAndArguments`? – Lepidopteron Oct 17 '17 at 14:37
  • No, @Lepidopteron, one cannot. But you could, conceivably, provide function-like *macros* for the users that wrap the real functions and append `(NSObject *) NULL` to the users' argument lists. That still won't protect you from misbehavior if variable arguments of non-pointer type are passed, however. Variadic functions place substantial responsibility on users to pass appropriate arguments; the compiler cannot check them. – John Bollinger Oct 17 '17 at 14:51
  • 1
    @Lepidopteron You can get a compiler warning if the list is not properly terminated by using the `NS_REQUIRES_NIL_TERMINATION` macro. However, as John pointed out, **this only works if all the args are all pointer types**. If you expect primitives to be passed as well (which seems pretty likely), you must obtain a count of the number of items to read. This can be done either by counting the format specifiers inside your function (that's how `printf`/`stringWithFormat:` do it), or by adding an explicit count parameter. – jscs Oct 17 '17 at 16:48
  • @JoshCaswell cool thank you Josh. You are right, I do also have primitive data types being passed along. The counting of format specifiers is already in place, to ensure the app would not crash if one would pass not enough values for the given format tho. E.g. I want to ensure the amount of given parameters does not exceed the amount of format specifiers in the format-string. Additionally, I wonder how Apple accomplished that kind of function in their NSLog statement, as it obviously also uses the va_list for NSLogv. Am I thinking too complicated? Anyhow, this discussion enlighted me a lot! – Lepidopteron Oct 17 '17 at 16:54
  • @Lepidopteron Glad it's helpful. `NSLog()` does the same thing: count the format specifiers, and stop calling `va_arg()` when you've pulled that many items off the list. – jscs Oct 17 '17 at 17:04
  • @JoshCaswell I stumbled over the possibility that there might be more format specifiers being provided than arguments passed, but I think I just have to implement it a bit smarter than either the minimum amount of va_args or format specifiers is taken into consideration. – Lepidopteron Oct 17 '17 at 17:11