On Sat, Feb 19, 2022 at 03:48:42AM +0100, Ævar Arnfjörð Bjarmason wrote: > >> > +} > >> > + > >> > +# If [AL]SAN is in effect we want to abort so that we notice problems. > >> > +prepend_var XSAN_OPTIONS : abort_on_error=1 > >> > >> XSAN_OPTIONS stands for "options that are common to all ?SAN", I > >> guess. > > > > I was also unclear on this. Looking around in the google/sanitizers > > repository, I don't see something called "XSAN_OPTIONS" mentioned > > anywhere (neither in documentation nor in the actual source code). > > > > Is this a convenience variable that we use to store options that are > > common to both ASAN_OPTIONS and LSAN_OPTIONS? If so, I am not sure the > > extra confusion is worth it, since it only contains abort_on_error=1. > > > > I guess it makes it (along with the prepend_var function introduced by > > this patch) possible for a caller to write XSAN_OPTIONS=... into their > > environment and then run a test script and have their settings feed into > > ASAN_OPTIONS and LSAN_OPTIONS. But I don't know that callers would find > > this super useful (or, at least not dramatically more convenient than > > setting both variables). > > > > I could be wrong, and I'm obviously biased towards my own usage of the > > ASAN/LSAN builds, but to me this patch might be clearer without the > > extra variable. > > Sorry, yes it's just a git.git invention to refer to "common LSAN and > ASAN' things. Perhaps GIT_ASAN_AND_ASAN_COMMON or something would be > much less confusing & better? Bikeshedding most welcome :) Yeah, I would be fine with something like GIT_ASAN_AND_LSAN_COMMON if it makes things easier. I think it definitely is clearer, and does make it easier to add new options to both. I probably wouldn't ever tweak its value myself, so I don't think I have a strong opinion here. If it were me, I'd probably just as soon duplicate setting `abort_on_error=1` in the ASan- and LSan-specific variables. But I don't have strong feelings here, so whatever makes the most sense to you is fine with me. Thanks, Taylor
