On Thu, Mar 05, 2020 at 05:56:35PM +0100, Tim Duesterhus wrote:
> @@ -3484,13 +3491,21 @@ stats_error_parsing:
> }
>
> else if (strcmp(args[0], "unique-id-header") == 0) {
> + char *copy;
> if (!*(args[1])) {
> ha_alert("parsing [%s:%d] : %s expects an argument.\n",
> file, linenum, args[0]);
> err_code |= ERR_ALERT | ERR_FATAL;
> goto out;
> }
> - free(curproxy->header_unique_id);
> - curproxy->header_unique_id = strdup(args[1]);
> + copy = strdup(args[1]);
> + if (copy == NULL) {
> + ha_alert("parsing [%s:%d] : failed to allocate memory
> for unique-id-header\n", file, linenum);
> + err_code |= ERR_ALERT | ERR_FATAL;
> + goto out;
> + }
> +
> + istfree(&curproxy->header_unique_id);
> + curproxy->header_unique_id = ist(copy);
> }
Not important at all and I'll take it like this, but since you now
have istdup() you could have simplified it further:
copy = istdup(ist(args[1]));
if (!isttest(copy) {
alert();
}
istfree(&curproxy->header_unique_id);
curproxy->header_unique_id = copy;
By the way this pointer swap pattern is particularly interesting because
it's used a lot in the config parser. It makes me think we could later
improve that by having :
/* frees ist and replaces it with a copy of str. Returns non-0 on success
* success or zero on allocation failure with the original left intact.
*/
int istreplace(struct ist *ist, const char *str)
{
struct ist new = istdup(ist(str));
if (!isttest(new))
return 0;
istfree(*ist);
*ist = new;
return 1;
}
Then at various places we could simply do :
if (!istreplace(&curproxy->foo, argv[1])) {
alert();
}
I just don't know how many candidate places we have for this :-)
Willy
