On Wed, 20 May 2026 at 21:44, Daniel Sahlberg <[email protected]> wrote:
> Den ons 20 maj 2026 kl 20:30 skrev <[email protected]>: > > > > Author: ivan > > Date: Wed May 20 18:29:58 2026 > > New Revision: 1934443 > > > > Log: > > Rewrite code to make it less error prone. No functional changes intended. > > > > * subversion/libsvn_subr/sysinfo.c > > (svn_sysinfo__linked_libs): Use separate scope for each lib. > > > > Modified: > > subversion/trunk/subversion/libsvn_subr/sysinfo.c > > > > Modified: subversion/trunk/subversion/libsvn_subr/sysinfo.c > > > ============================================================================== > > --- subversion/trunk/subversion/libsvn_subr/sysinfo.c Wed May 20 > 17:04:24 2026 (r1934442) > > +++ subversion/trunk/subversion/libsvn_subr/sysinfo.c Wed May 20 > 18:29:58 2026 (r1934443) > > @@ -146,56 +146,72 @@ svn_sysinfo__character_encoding(apr_pool > > const apr_array_header_t * > > svn_sysinfo__linked_libs(apr_pool_t *pool) > > { > > - svn_version_ext_linked_lib_t *lib; > > - apr_array_header_t *array = apr_array_make(pool, 7, sizeof(*lib)); > > - int lz4_version = svn_lz4__runtime_version(); > > - > > - lib = &APR_ARRAY_PUSH(array, svn_version_ext_linked_lib_t); > > - lib->name = "APR"; > > - lib->compiled_version = APR_VERSION_STRING; > > - lib->runtime_version = apr_pstrdup(pool, apr_version_string()); > > + apr_array_header_t *array = > > + apr_array_make(pool, 7, sizeof(svn_version_ext_linked_lib_t)); > > + > > + { > > + svn_version_ext_linked_lib_t *lib = apr_array_push(array); > > + lib->name = "APR"; > > + lib->compiled_version = APR_VERSION_STRING; > > + lib->runtime_version = apr_pstrdup(pool, apr_version_string()); > > + } > > Would it make sense to factor this out to a separate function, instead > of duplicating this code here... > While we could deduplicate the code using one of the options below, I actually prefer leaving the original version as is. Pseudocode option A, with an init helper: [[[ static void init_linked_lib(svn_version_ext_linked_lib_t *lib, const char *name, const char *compiled_version, const char *runtime_version) { lib->name = name; lib->compiled_version = compiled_version; lib->runtime_version = runtime_version; } .... init_linked_lib(apr_array_push(array), "APR", APR_VERSION_STRING, apr_pstrdup(pool, apr_version_string())); ]]] Pseudocode option B, with a combined add/construct helper: [[[ static void add_linked_lib(apr_array_header_t *array, const char *name, const char *compiled_version, const char *runtime_version) { svn_version_ext_linked_lib_t *lib = apr_array_push(array); lib->name = name; lib->compiled_version = compiled_version; lib->runtime_version = runtime_version; } .... add_linked_lib(array, "APR", APR_VERSION_STRING, apr_pstrdup(pool, apr_version_string())); ]]] This is because neither of these options seem to create a particularly useful abstraction. They also hide the memory management details, making it harder for a reader to tell who is responsible for duplicating data into the result pool. -- Ivan Zhakov
