Small message buffer optimization for SDL_log and SDL_assert

[eloj]

I saw the discussion on 2a42952 and subsequent commits, and thought this sort of solution could be another approach worth trying.

After forgetting I had to copy the va_list I spent way too much time on this... but that's on me, if you don't like the approach it goes in the bin.

Pros:

• Stack size worry goes away.
• No malloc for small messages.
• (Could in theory support any-size message)
• No mutex required (unless I'm missing something?)

Cons:

• More complicated code.
• malloc + extra vsnprintf for large messages.
• ... but in a pinch you could set SDL_MAX_LOG_MESSAGE_STACK to SDL_MAX_LOG_MESSAGE and avoid the extra allocation.

[icculus]

I think this is a good idea, with some tweaks, but the mutex needs to stay regardless.

[slouken]

I like the idea that we're no longer limited on log message size. Let's do it! :)

[eloj]

I appreciate the review, and I have pushed changes addressing Ryan's comments.

As for being limited on size, do you want me to actually remove SDL_MAX_LOG_MESSAGE and the truncation of messages, truly allowing 'unlimited' length log lines? It'd probably simplify the code to just malloc exactly what we need, but I sort of feel like the truncation is justified.

Starting to second-guess myself, maybe this isn't worth it and using a static buffer with locks around it is better overall.

Please be very specific in what you want, and I'll get on it.

[slouken]

Let's use a static buffer for messages <= 256 bytes, and malloc for longer lines. I'm not too worried about the unbounded pointer case, the likelihood of not getting a zero for a long time is pretty low, and malloc() will fail for really really long lines. We free the pointer almost immediately, so I'm not worried about introducing any issues with consuming too much memory here.

[eloj]

Okay, I'll rework the code to no longer truncate log messages. Same for the assert code?

[eloj]

I have pushed the changes to allow for 'unbounded' log and assert messages.

I have not touched this Apple code in SDL_LogOutput. I guess I'll do the same thing there, unless someone objects. I'll push this change later today.

       char *text;
        /* !!! FIXME: why not just "char text[SDL_MAX_LOG_MESSAGE];" ? */
        text = SDL_stack_alloc(char, SDL_MAX_LOG_MESSAGE);
        if (text) {
            SDL_snprintf(text, SDL_MAX_LOG_MESSAGE, "%s: %s", SDL_priority_prefixes[priority], message);
            SDL_NSLog(text);
            SDL_stack_free(text);
            return;
        }

[slouken]

I have not touched this Apple code in SDL_LogOutput. I guess I'll do the same thing there, unless someone objects. I'll push this change later today.

       char *text;
        /* !!! FIXME: why not just "char text[SDL_MAX_LOG_MESSAGE];" ? */
        text = SDL_stack_alloc(char, SDL_MAX_LOG_MESSAGE);
        if (text) {
            SDL_snprintf(text, SDL_MAX_LOG_MESSAGE, "%s: %s", SDL_priority_prefixes[priority], message);
            SDL_NSLog(text);
            SDL_stack_free(text);
            return;
        }

I'll go ahead and fix this.

[slouken]

@icculus, do you have a preference whether the 256 byte buffer is static or on the stack?
@eloj, if the buffer becomes static, you'll need to move the lock around the access to it.

[icculus]

Eloj convinced me; 256 bytes is good, and let's stack allocate it and see what happens.

[slouken]

@eloj, I think this PR is good to go.