Project

General

Profile

Actions

Feature #2458

closed

Simplify creating formatted buffers

Added by brarcher over 11 years ago. Updated 2 months ago.

Status:
Wontfix
Priority:
Normal
Category:
-
Target version:
-
ASK QUESTIONS IN Forums:
No

Description

I noticed that there was no easy way to append to a buffer with a defined format similar to snprintf. For example, network_init in network.c there is a section which creates a buffer like so:

buffer_copy_string_buffer(b, srv->srvconf.bindhost);
buffer_append_string_len(b, CONST_STR_LEN(":"));
buffer_append_long(b, srv->srvconf.port);

It would be a lot easier if there was one buffer_append function which would accept a variable number of arguments and create a buffer similar to how the log_error_write function works. For example:

buffer_append_format(b, "bsd",srv->srvconf.bindhost, ":", srv->srvconf.port);

Attached are two patches. One implements the buffer_append_format in buffer.c, modeled by the log_error_write function. The second uses this to replace a few calls in network_init. (Additionally, I noticed that the buffer being replaced could be leaked if network_server_init fails; I fix that as well).

This change does not result in any unit tests failing which did not previously fail. Namely, mod-cgi.t and request.t had failures before the change.


Files

buffer_append_format.patch (2.08 KB) buffer_append_format.patch brarcher, 2012-11-14 14:56
Actions #1

Updated by darix over 11 years ago

  • Status changed from New to Need Feedback
3 questions:
  1. this patch is for 1.4.x?
  2. what do you need it for?
  3. could it be converted to use the format string style from 1.4 instead of the printf style?
Actions #2

Updated by brarcher over 11 years ago

1) This patch is for 1.5.x. Forgot to check the "Missing in 1.5.x" box. Oops.
2) The reason I propose adding in buffer_append_format is to make appending to buffers simpler; e.g. to reduce complexity and code size. The replacement in network.c is only one example. If accepted, I could submit patches to replace other parts of the code that use multiple buffer_append_* calls as I encounter them.
3) I am not sure what the format style from 1.4 is. The patch I have submitted uses the format style in log_error_write, which is not printf, but does accept a variable number of arguments. Perhaps mentioning snprintf was misleading.

Actions #3

Updated by darix over 11 years ago

as 1.5 is more or less a dead branch ... would you consider to put your development efforts into lighttpd2 instead?

Actions #4

Updated by brarcher over 11 years ago

Hm, this is awkward...

I figured that the trunk of the lighttpd repository was where ongoing development was. I now see the lighttpd2 repo.

Please disregard this patch in that case.

Actions #5

Updated by stbuehler over 11 years ago

  • Tracker changed from Bug to Feature
  • Status changed from Need Feedback to Wontfix
Actions #6

Updated by gstrauss 2 months ago

  • ASK QUESTIONS IN Forums set to No

lighttpd 1.4.x moved to prefer standard printf-like format strings to allow compilers such as gcc and clang to validate the format strings.

Not part of lighttpd, but this is what a printf-like buffer_append_snprintf() routine might look like (untested):

__attribute_format__((__printf__, 2, 3))                                                           
unsigned int
buffer_append_snprintf (buffer *b, const char *fmt, ...)
{
    unsigned int n;                                                                                
    va_list ap;
    va_start(ap, fmt);

    va_list apn;                                                                                   
    va_copy(apn, fmt);
    n = (unsigned int)vsnprintf(NULL, 0, fmt, apn);                                                
    va_end(apn);

    if ((int)n >= 0) /*(buffer_extend() calls abort() if n too large)*/                            
        vsnprintf(buffer_extend(b, n), n, fmt, ap);

    va_end(ap)                                                                                     
    return n;
}

Actions

Also available in: Atom