On Mon, Jun 28, 2021 at 9:19 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > Convert the config reading code in upload-pack.c to use the new > "startup_config" callback when we're using the v2 protocol, and > lightly fake up the same when we're using v0 and v1. I'm not a fan creating more global variables rather, but I'll it leave to someone else to decide what's best. This change looks consistent with its predecessor, and OK modulo a few nits below. > Thus it's clear that the "allow_uor" passed to functions like > "is_our_ref()" is constant after startup (usually it'll never change > for a given server's configuration, or change once). > > This requires a very light compatibly layer with the serve.c callback typo. > +/* > + * "Global" configuration that won't be affected by the type of > + * request we're doing, or by other request data in "struct > + * upload_pack_data" below. > + */ > +static int v1_have_startup_config; > +static int config_keepalive = 5; could this include a unit? config_keepalive_secs , I think? > + int ret; > + ret = upload_pack_startup_config(var, value, data); > + if (ret) > + return ret; > + ret = upload_pack_startup_config_v2_only(var, value, data); > + if (ret) > + return ret; Neither of these config readers ever return -1. Do you foresee this ever happening? Or could this return void? > + if (!v1_have_startup_config++) > + git_config(upload_pack_startup_config, NULL); See comment in previous patch about ++ for boolean values. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
