I'm not sure "memprintf" will succeed in case of OOM. I'll check
ср, 11 дек. 2024 г. в 04:39, Willy Tarreau <[email protected]>:
> Hi Ilya,
>
> Thanks for these patches. Please be careful, most parsing or init
> functions that return ERR_ALERT are expected to have either printed
> the alert or produced it in the "err" field. Sadly there's no general
> rule to know which one uses what, generally it's needed to see what
> other parts are doing in the same function. All generic keyword
> parsers at least use memprintf(). I've checked them and here are the
> needed changes for the 4 patches:
>
> On Tue, Dec 10, 2024 at 01:23:08PM +0100, Ilia Shipitsin wrote:
> > diff --git a/src/check.c b/src/check.c
> > index 29a86b7dd..e636148b6 100644
> > --- a/src/check.c
> > +++ b/src/check.c
> > @@ -1667,6 +1667,8 @@ static int start_checks()
> > /* 0- init the dummy frontend used to create all checks sessions */
> > init_new_proxy(&checks_fe);
> > checks_fe.id = strdup("CHECKS-FE");
> > + if ( !checks_fe.id )
> (side note: ^ ^ please do not add these unneeded spaces).
>
> This block should emit the error directly using ha_alert(), please see
> around line 1715, there's one already, I think it should be something
> like:
>
> ha_alert("Out of memory creating the checks frontend.\n");
>
> > + return ERR_ALERT | ERR_FATAL;
> > checks_fe.cap = PR_CAP_FE | PR_CAP_BE;
> > checks_fe.mode = PR_MODE_TCP;
> > checks_fe.maxconn = 0;
>
>
> > diff --git a/src/listener.c b/src/listener.c
> > index 5f3a98b4a..5fb610a88 100644
> > --- a/src/listener.c
> > +++ b/src/listener.c
> > @@ -2369,8 +2369,11 @@ static int bind_parse_name(char **args, int
> cur_arg, struct proxy *px, struct bi
> > return ERR_ALERT | ERR_FATAL;
> > }
> >
> > - list_for_each_entry(l, &conf->listeners, by_bind)
> > + list_for_each_entry(l, &conf->listeners, by_bind) {
> > l->name = strdup(args[cur_arg + 1]);
> > + if (!l->name)
>
> This block should do:
>
> memprintf(err, "'%s %s' : out of memory", args[cur_arg],
> arg[cur_arg + 1]);
>
> > + return ERR_ALERT | ERR_FATAL;
> > + }
> >
> > return 0;
> > }
>
>
> > diff --git a/src/mux_h1.c b/src/mux_h1.c
> > index 7eb18133d..0206dcbd0 100644
> > --- a/src/mux_h1.c
> > +++ b/src/mux_h1.c
> > @@ -5663,7 +5663,10 @@ static int
> cfg_parse_h1_headers_case_adjust_file(char **args, int section_type,
> > }
> > free(hdrs_map.name);
> > hdrs_map.name = strdup(args[1]);
> > - return 0;
> > + if (!hdrs_map.name)
>
> This block should do:
>
> memprintf(err, "'%s %s' : out of memory", args[0], arg[1]);
>
> > + return -1;
> > +
> > + return 0;
> > }
>
>
> > diff --git a/src/debug.c b/src/debug.c
> > index d052f5b63..2c5e35553 100644
> > --- a/src/debug.c
> > +++ b/src/debug.c
> > @@ -2048,6 +2048,8 @@ static int debug_parse_cli_memstats(char **args,
> char *payload, struct appctx *a
> > else if (strcmp(args[arg], "match") == 0 && *args[arg +
> 1]) {
> > ha_free(&ctx->match);
> > ctx->match = strdup(args[arg + 1]);
> > + if (!ctx->match)
> > + return 1;
>
> For this one, it's a runtime CLI keyword parser, which is more delicate
> than the other ones (and a *real* problem as it's possible to crash the
> process at runtime while using the command). This one should do it:
>
> return cli_err(appctx, "Out of memory.\n");
>
> and you can drop the return 1.
>
> Thank you!
> Willy
>
