kristapsdz / kcgi

minimal CGI and FastCGI library for C/C++
https://kristaps.bsd.lv/kcgi
ISC License
271 stars 40 forks source link

kxml_pushattrs() fails if more than one pair of arguments are provided #69

Closed ghost closed 4 years ago

ghost commented 4 years ago

I am running kcgi 0.10.11 installed from package on openbsd. I've been using the khtml functions for years but recently took a look at the kxml functions to try to create an svg image. I hit a snag when trying to add multiple attributes to the root element.

Here is the setup:

resp_open(kr, KHTTP_200, "image/svg+xml");

const char* SVG_ELEMS[] = {
  "svg",
};

enum SVGs {
  SVG_SVG,
  SVG__MAX
};

kxml_open(&xr, kr, SVG_ELEMS, SVG__MAX);
kxml_prologue(&xr);

kxml_pushattrs(&xr, SVG_SVG,
               "xmlns",   "http://www.w3.org/2000/svg",
               //"version", "1.2",                                                                                                                                                                                                                                                                                        
               //"baseProfile", "tiny",                                                                                                                                                                                                                                                                                   
               //"width", "5cm",                                                                                                                                                                                                                                                                                          
               //"height", "4cm",                                                                                                                                                                                                                                                                                         
               //"viewBox", "0 0 100 100",                                                                                                                                                                                                                                                                                
               0);

where resp_open() does this:

void resp_open(struct kreq *kr, enum khttp http, std::string mime) {
    khttp_head(kr, kresps[KRESP_STATUS], "%s", khttps[http]);
    khttp_head(kr, kresps[KRESP_CONTENT_TYPE], "%s", mime=="" ? kmimetypes[kr->mime] : mime.c_str());
    khttp_body(kr);
}

The above code will produce this:

<?xml version="1.0" encoding="utf-8" ?><svg xmlns="http://www.w3.org/2000/svg"></svg>

Okay, so far so good, but if I uncomment a line to add another attribute, kxml_pushattrs() does not return and the resulting page is solid white. I did some searching for examples of this in use and found kcaldav, but from what I can see, I am doing the same thing.

If you can see what dumb mistake I am probably making, please let me know. If not, what else can I do to track down this issue?

Thanks!

ghost commented 4 years ago

I found the issue - i replaced the '0' at the end of the call with 'NULL' and it worked. I thought NULL was defined as 0 so I really don't get why this matters, but problem solved!

kristapsdz commented 4 years ago

Thought I'd actually explain this because it'll catch you elsewhere too.

Anywhere you're using variable arguments (functions that have "..."), the variable number of arguments must have an explicitly stated type. Or really, width in bytes. Because the function implementing the function is going to step through the arguments byte by byte. So if you have foo(...) where the variable list should be integers, foo(int, int, int) is implementede to look at the first sizeof(int), then the next, etc.

So if foo(...) is stated to accept pointers with NULL as the last pointer, passing 0 is incorrect. The reason being that the zero may be (for instance) an integer (32 bits) while the pointer may be 64 bits. That means that instead of a NULL, you're really passing a zero plus whatever comes for the next 32 bits. If your integer length is the same as your pointer length, it's correct but cause you got lucky.

The only function I can think of in the wild where this is important is execlp(3) and friends, where the arguments must be terminated by a NULL pointer. Same caveat there if you pass a zero.

ghost commented 4 years ago

That makes sense. Thanks for the explanation!