Re: I'd like to contribute but I don't know how...
On Thu, Feb 22, 2018 at 09:25:39PM +, Alex Arvelaez wrote: > On Thu, Feb 22, 2018 at 04:12:14AM +, Alex Arvelaez wrote: > > On Wed, Feb 21, 2018 at 10:20:07PM -0500, valdis.kletni...@vt.edu wrote: > > > On Thu, 22 Feb 2018 02:33:08 +, 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.html > > > > > > > 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? > > > > I went ahead and used log10() from -lm, everything seems to work > properly from what I can see. I made a patch with my changes should > I send that to the linux-kernel mailing list for feedback? Documentation/process/ is your friend. Tobin ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: I'd like to contribute but I don't know how...
On Thu, Feb 22, 2018 at 04:12:14AM +, Alex Arvelaez wrote: > On Wed, Feb 21, 2018 at 10:20:07PM -0500, valdis.kletni...@vt.edu wrote: > > On Thu, 22 Feb 2018 02:33:08 +, 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.html > > > > > 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? > I went ahead and used log10() from -lm, everything seems to work properly from what I can see. I made a patch with my changes should I send that to the linux-kernel mailing list for feedback? Regards, Alex ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: I'd like to contribute but I don't know how...
On Wed, Feb 21, 2018 at 10:20:07PM -0500, valdis.kletni...@vt.edu wrote: > On Thu, 22 Feb 2018 02:33:08 +, 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.html > > > 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 ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: I'd like to contribute but I don't know how...
On Wed, Feb 21, 2018 at 10:20 PM, wrote: > On Thu, 22 Feb 2018 02:33:08 +, Alex Arvelaez said: >> ... >> >> I'd like to contribute to the linux kernel eventually but I'm not sure > > https://lists.kernelnewbies.org/pipermail/kernelnewbies/2017-April/017765.html This comes up so frequently the list might consider sending a FAQ once a month with links to the answers. Its more useful than Mailman emailing plaintext passwords :) Jeff ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Re: I'd like to contribute but I don't know how...
On Thu, 22 Feb 2018 02:33:08 +, 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.html > 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? Do you understand *why* '28' was used? And why they didn't just go ahead and use the perfectly usable log10() found in -lm ? 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. For extra bonus points, figure out how long that FIXME has been there. And why. (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. pgp6dQqutTVmu.pgp Description: PGP signature ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
I'd like to contribute but I don't know how...
Hello, I'd like to contribute to the linux kernel eventually but I'm not sure 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... */ 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... Regards, Alex ___ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies