On Mon, Jul 30, 2007 at 11:19:28PM +0200, Daniel Stenberg wrote:
> Here's my updated patch, almost twice as big as before but I like this
> approach better. I'm ready to commit it like this unless someone speaks
> against this concept in a convincing manner! ;-)

My only objection is that it forces all option strings to be treated
strangely and differently compared to other option data, because of
a constraint at the program interface and not because of any internal reason.
Someone seeing data->set.str[STRING_DEVICE] will immediately think that
str is an array because it needs to be an array, not because of the real
reason--that it's just somewhat more convenient for it to be an array
at the program interface. The data also drops from being a first-class
member of the set array to a second class member, segregated by type.
The old name data->set.device makes it clear that the data is on the same
level as other option data, alongside data->set.localport.

Putting the strings into a union something like:

union {
  struct str {
    char *device;
    char *set_referer;
    char *useragent;
    ...
  } str;
  char *allstrings[sizeof(struct str)/sizeof(char *)];
} strs;

solves this half way, allowing access with data->set.strs.str.device while
still allowing access through an array, so even if it's portable it's
still a bit ugly. It could also be solved by using macros to map into
the array using the old names, but that starts getting even uglier for
not a good enough reason.

I suppose the string array will have to do. Here are a few minor things I
spotted in that patch.

> + char *dev = data->set.str[STRING_DEVICE];

This could be a const char *.

> + /* -- end of ,trings -- */

Typo.

> + STRING_SSL_CAFILE, /* cerficate file to verify peer against */

Typo: cerficate

> + /* Copy src->set into dst->set, taking care of duplifying dynamic strings */

Typo: duplifying => duplicating

> + /* If a failure occured, freeing has to be performed externally. */

Typo: occured

>>> Dan

-- 
http://www.MoveAnnouncer.com              The web change of address service
          Let webmasters know that your web site has moved