Further refactoring towards steamrt/tasks#595
-
pv-adverb: Factor out parsing of --ld-preload, --ld-audit arguments
This allows it to have test coverage.
-
pv-wrap: Use PvPreloadVariableIndex to represent LD_PRELOAD vs. LD_AUDIT
Convert to a concrete pv-adverb option name at the last possible moment.
This will help us to centralize the code to map to/from pv-adverb command-line options.
-
pv-wrap: Use an index into pv_multiarch_tuples to represent architecture
This will help us to centralize the code that maps to/from pv-adverb command-line options.
-
pv-wrap: Build adverb arguments from PvAdverbPreloadModule structs
This means all the parsing and serialization is in one place, and we can round-trip between the struct representation and the command-line option representation.
-
pv-adverb: Factor out processing of each PvAdverbPreloadModule
No functional change. Best reviewed with
git show --ignore-space-change
.
As usual with refactoring, best reviewed a commit at a time.
Merge request reports
Activity
requested review from @denittis
assigned to @smcv
added Awaiting Review Tech debt pressure-vessel labels
mentioned in merge request !784 (merged)
mentioned in merge request !785 (merged)
mentioned in merge request !780
198 * 199 * Returns: %TRUE if the argument was parsed successfully 200 */ 201 gboolean 202 pv_adverb_preload_module_parse_adverb_cli (PvAdverbPreloadModule *self, 203 const char *option, 204 PvPreloadVariableIndex which, 205 const char *value, 206 GError **error) 207 { 208 g_auto(GStrv) parts = NULL; 209 const char *architecture = NULL; 210 gsize abi_index = PV_UNSPECIFIED_ABI; 211 gsize abi; 212 213 g_return_val_if_fail (self->name == NULL, FALSE); This is always a trade-off: anything we assert about in places like this is already meant to be a "can't happen" situation, and there's such a thing as having too many preconditions (making it harder to see the important ones).
In public API of a library I'd definitely do as you suggest, but in this internal function I think my inclination is to not. If
self
can somehow be null, we'll find out very quickly anyway, when it segfaults.
261 261 self->abi_index = abi_index; 262 262 return TRUE; 263 263 } 264 265 gchar * 266 pv_adverb_preload_module_to_adverb_cli (PvAdverbPreloadModule *self) 267 { 268 g_autoptr(GString) buf = NULL; 269 const PvPreloadVariable *variable; 270 271 g_return_val_if_fail (self->index_in_preload_variables < G_N_ELEMENTS (pv_preload_variables), NULL); removed Awaiting Review label
mentioned in commit 904d6432