On Thu, Dec 22, 2011 at 04:36:43PM +0000, Colin Hogben wrote:
> The attached patches improve the output of curl's --libcurl option by
> generating code to build curl_httppost and curl_slist lists, and use
> symbolic names for enum and flag values.

That sounds very useful.

> +#define my_setopt_enum(x,y,n,z) do { \
> + res = tool_setopt_enum(x, config, #y, y, n##_namevalues, z); \
> + if(res) \
> + goto show_error; \
> +} WHILE_FALSE

You should be able to eliminate the n parameter by using y instead:

#define my_setopt_enum(x,y,z) do { \
  res = tool_setopt_enum(x, config, #y, y, y##_namevalues, z); \

(also in my_setopt_flags).

> +/* Escape string to C string syntax. Return NULL if out of memory.
> + * Is this correct for those wacky EBCDIC guys? */
> +static char *c_escape(const char *str)
> +{
> + size_t len = 0;
> + const char *s;
> + unsigned char c;
> + char *escaped, *e;
> + /* First pass - work out how much space we need */
> + for(s=str; (c=*s) != '\0'; s++) {
> + if(c=='\n' || c=='\r' || c=='\t' || c=='\\' || c=='"') {
> + len += 2;
> + }
> + else if(! isprint(c)) {
> + len += 4;
> + }
> + else
> + len ++;
> + }

I'm always uncomfortable seeing duplicated code, and this loop is basically
a subset of the one below. These strings are going to be pretty short,
so it would be quicker and smaller (at the cost of more heap) to just
use the platform-optimized strlen() function and malloc a worst-case
buffer size (strlen(str)*4+1). Environments where the user will be calling
--libcurl shouldn't have issues with memory.

But, since this stuff won't be required by embedded (and many other)
applications, it would be useful to make all the --libcurl code
conditionally compiled so it can be removed altogether when unneeded.

> +/* setopt wrapper for bit mask */
> +CURLcode tool_setopt_flags(CURL *curl, struct Configurable *config,
> + const char *name, CURLoption tag,
> + const NameValue *nvlist, long lval)
> +{
> + CURLcode ret = CURLE_OK;
> + bool skip = FALSE;
> +
> + ret = curl_easy_setopt(curl, tag, lval);
> + if(!lval)
> + skip = TRUE;
> +
> + if(config->libcurl && !skip && !ret) {
> + /* we only use this for real if --libcurl was used */
> + char preamble[80]; /* should accommodate any symbol name */
> + long rest = lval; /* bits not handled yet */
> + const NameValue *nv = NULL;
> + snprintf(preamble, sizeof(preamble),
> + "curl_easy_setopt(hnd, %s, ", name);
> + for(nv=nvlist; nv->name; nv++) {
> + if((nv->value & ~ rest) == 0) {
> + /* all value flags contained in rest */
> + rest &= ~ nv->value; /* remove bits handled here */
> + CODE3("%s(long)%s%s",
> + preamble, nv->name, rest ? " |" : ");");
> + if(!rest)
> + break; /* handled them all */
> + /* replace with all spaces for continuation line */
> + sprintf(preamble, "%*s", strlen(preamble), "");
> + }
> + }
> + /* If any bits have no definition, output an explicit value.
> + * This could happen if new bits are defined and used
> + * but the NameValue list is not updated. */
> + if(rest)
> + CODE2("%s%ldL);", preamble, rest);

preamble here could be all spaces if that path in the for loop were
previously taken.

> diff --git a/tests/data/test1400 b/tests/data/test1400
> new file mode 100644
> index 0000000..67b2745
> --- /dev/null
> +++ b/tests/data/test1400
> @@ -0,0 +1,92 @@
> +<testcase>
> +<info>
> +<keywords>
> +HTTP
> +libcurl

Since this is the option being described, it should really be --libcurl

>>> Dan
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2011-12-26