Matthew DeVore <matvore@xxxxxxxxxx> writes: > - filter_options->filter_spec = strdup(core_partial_clone_filter_default); > + if (!filter_options->filter_spec.buf) > + strbuf_init(&filter_options->filter_spec, 0); This part made me go "Huh?" a bit. Do we document that .buf==NULL means an uninitialized strbuf that is safe to run strbuf_init() on? I do not mind that as a general convention, and it may even be a useful one (i.e. it allows you to calloc() a structure with an embedded strbuf in it and the "if .buf==NULL, call strbuf_init() lazily" can become an established pattern), but at the same time it feels a bit brittle. Such a convention forces everybody who might want to use such an embedded strbuf to first check .buf==NULL and lazily initialize it---and at some point when the embedded strbuf to be used by enough codepaths, it would make the code more robust by giving up on the lazy initialization (iow, when *filter_options is initialized, run strbuf_init() on its .filter_spec field).
