On Wed, Feb 21, 2018 at 10:20:07PM -0500, valdis.kletnieks@vt.edu wrote:
On Thu, 22 Feb 2018 02:33:08 +0000, Alex Arvelaez said:
Hello,
I'd like to contribute to the linux kernel eventually but I'm not sure
https://lists.kernelnewbies.org/pipermail/kernelnewbies/2017-April/017765.ht...
how, I grabbed a copy of the source code and I found a FIXME that looks like I could fix:
/* File: /usr/src/linux/tools/perf/util/string.c * FIXME: replace this with an expression using log10() when we * find a suitable implementation, maybe the one in the dvb drivers... */
Step 0: Verify that the comment still matches the code, *and* that the change is still desired. Hint: Why do they want log10()? What does the current code do? What, if anything, will break if you change it?
It's in a function that creates boolean expressions from an array of integers and it looks they hardcoded 28 bytes per each number. I think the intent of using log10() is to only allocate as many bytes as the largest number so expressions with lower numbers(in magnitude) would use less memory.
Do you understand *why* '28' was used? And why they didn't just go ahead and use the perfectly usable log10() found in -lm ?
I'll look into this, I think they are including stdlib.h so it's a very good point. I think they used 28 to be on the safe side 28 bytes gives room for 27 digits per number. They also put the formula they're trying to approximate: /* "%s == %d || " = log10(MAXINT) * 2 + 8 chars for the operators */
For bonus points, understand the code's behavior (or misbehavior) on 32 versus 64 bit architectures, and whether or not that's actually a problem.
I don't think that would be a problem but I have to take a closer look at the implementation of log10().
For extra bonus points, figure out how long that FIXME has been there. And why.
The commit is from 2015, I couldn't find much about it on the lkml.org archives. According the commit message the function's used to generate to generate expressions to filter syscalls when tracing them: perf -e read,write,fork //we only track specified syscalls in this case
(Note that some of these are things that I don't know the answer to offhand. ;)
I found the implementation they mention, is it okay to just copy paste it into the file? I'm not sure where else to ask this kind of question...
If it's going to be used in more than one place, it should be refactored into a function, and both usage sites reworked to call the function rather than inline.
It's only a couple functions, they defined log10() in terms of log2(). I don't really know how to avoid inlining though, I'll look into it.
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies