[DTrace-devel] [PATCH v2 2/2] tests, actions: fix tst.symmod.sh

[Kris Van Hees]

We still need a v3 posted, I think?

On Tue, Feb 13, 2024 at 04:02:25PM +0000, Nick Alcock wrote:
> On 13 Feb 2024, Eugene Loh via DTrace-devel told this:
> 
> > The comment about never using shuf does not belong in the commit message.
> > 
> > The indentation in the file is with tabs, so that style should be preserved.
> 
> ... it was? Whoops :)
> 
> > Instead of inflooping (in flooping?), how about "infinitely looping".? (I do find some occurrences of "inflooping" on the internet,
> > but might as well be less slangy?)
> 
> I find 'infinitely looping' clunky. "Looping infinitely" is even
> clunkier. Inflooping is an established term of art, frankly.
> (Irrelevant given the below anyway.)
> 
> > Anyhow, instead of the shuffle approach, how about "awk 'NF == 4' /proc/kallsyms" to a temp file and use that?
> 
> Oh yeah that would work, though I don't see what's wrong with using
> shuf...
> 
> > But even that won't be enough.? Once you allow "any" symbol from
> > /proc/kallsyms, you risk encountering symbols that DTrace doesn't know
> > about.? Stuff like __ksymtab* and __kstrtab* and others.? I think we
> > filter that stuff out in dt_modsym_addsym().? The same filtering is
> > done in test/unittest/consumer/tst.symbols.c.? It's the headache one
> > saves by going after a specific symbol like ksys_write.
> 
> ... but doing *that* seems like it would render the test nearly useless.
> 
> the fact that tst.symmod.sh as I found it was broken for *all modules*
> even though much of its code was meant to handle them suggests that
> whatever it was intended to test, it wasn't actually testing it. (There
> are no comments describing what it's for, which makes it hard to figure
> out how to preserve that property when changing it.)
> 
> (I mean among other things it was carefully selecting a guaranteed
> non-modular symbol, so what all this code to figure out what module it
> was in was for, even if it had worked, I have no idea...)
>