Thin Air

<< Prev: The Linker       Next: kzalloc Crash >>

It was clear I wasn't going to make any progress without finding these missing functions. Eventually, since they were declared in iwl-debug.h, I got around to looking in iwl-debug.c. And this is what I found:

#define __iwl_fn(fn)                                       \
void __iwl_ ##fn(struct device *dev, const char *fmt, ...) \
{                                                          \
    struct va_format vaf = {                               \
        .fmt = fmt,                                        \
    };                                                     \
    va_list args;                                          \
                                                           \
    va_start(args, fmt);                                   \
    vaf.va = &args;                                        \
    dev_ ##fn(dev, "%pV", &vaf);                           \
    trace_iwlwifi_ ##fn(&vaf);                             \
    va_end(args);                                          \
}

__iwl_fn(info)
IWL_EXPORT_SYMBOL(__iwl_info);

This, my friends, is the bytecode manipulation I'm familiar with from Java, only performed in C.  :)   

OK, also performed at compile time.

Nevertheless, it is a macro that generates a function out of thin air.

The idea is, you call the macro like in the second-to-last line above with a parameter info and it generates the function __iwl_info. Then the last line makes sure the function that's never actually plainly stated in the source code is callable. Repeat those last lines a couple times to declare a couple similar functions (_warn, _crit).

OK, so two questions:

1. Does it work?
2. Why? Just... Why?

And the answers:

1. No.
2. So you can make a few nearly-identical functions without wasting keystrokes...?

It doesn't work because the functions that do the meaty work (e.g. dev_info and trace_iwlwifi_info) don't exist. The first is a Linux API call and the second I haven't ported yet. Also, the va_format struct is (I believe) Linux-specific. So basically the whole thing doesn't work.

Really, I just want to make an IOLog call with a prefix on the message like INFO, DEBUG, WARN, etc.

Why did someone write it this way to begin with? (This one is multiple-choice.)

• A. It seemed really clever
• B. It saves space in the executable
• C. It saved lines of code in the source
• D. It made it hard for others to follow, until they really think about it, at which point they might appreciate option A.

The only way I can see arguing in favor of this approach is if the answer was option B. But since the macro is processed at compile time and thus actually generates separate functions every time it's called, I think the answer is actually A-C-D. In other words, everything else.

Alternatives, then.

I could fix up all the generic code in the macro, but it would involve defining additional functions like emit_log_dev and emit_log_info that had the specific message handling logic I wanted, so the code in the macro could construct an appropriate call. And I already don't like the macro approach. Why go through this if I have to write the specific log-level handler functions anyway?

So if I wanted to save lines of source code, I could create one master function with a switch on a parameter to control which dev_* (and later trace_iwlwifi_*) functions are called, and then define the three tiny functions that call the master function with appropriate arguments. But even when I fudged the message prefixes to be the same length, half the logic is in the handling of the printf-style arguments, and that would be a mess to try to pick out and pass and use in a master function (I suppose necessitating a struct like va_format). I don't think there would be any savings.

So I'm going to go with the option of just writing three functions. You know, not being overly clever, just writing code that does what I mean for it to do in the functions that are supposed to be called. It's still complicated by the fact that these functions take a variable number of arguments to support printf-style messages, but none of them are that long.

And I get this:

void __iwl_info(struct device *dev, const char *fmt, ...) {
    char buffer[200] = "AppleIntelWiFiMVM INFO ";
    char *remainder = &buffer[23];
    va_list args;
    va_start(args, fmt);
    vsnprintf(remainder, 177, fmt, args);
    va_end(args);
    IOLog(buffer);
}
void __iwl_crit(struct device *dev, const char *fmt, ...) {
    char buffer[200] = "AppleIntelWiFiMVM CRIT ";
    char *remainder = &buffer[23];
    va_list args;
    va_start(args, fmt);
    vsnprintf(remainder, 177, fmt, args);
    va_end(args);
    IOLog(buffer);
}
void __iwl_warn(struct device *dev, const char *fmt, ...) {
    char buffer[200] = "AppleIntelWiFiMVM WARN ";
    char *remainder = &buffer[23];
    va_list args;
    va_start(args, fmt);
    vsnprintf(remainder, 177, fmt, args);
    va_end(args);
    IOLog(buffer);
}

Oh yeah, here's the punch line: The __iwl_dbg and __iwl_err functions take different parameters and have a couple extra lines of logic — so the macro didn't work for them anyway. That means of five short and nearly identical functions, there were two written out, and three hidden away in a weird macro.

Too clever for me.

<< Prev: The Linker       Next: kzalloc Crash >>