Date: Fri, 25 Jun 2021 15:28:25 +0800 From: Yun Zhou <yun.zhou@...driver.com> To: Steven Rostedt <rostedt@...dmis.org> Cc: linux-kernel@...r.kernel.org, kernel-hardening@...ts.openwall.com, ying.xue@...driver.com, "Li, Zhiquan" <Zhiquan.Li@...driver.com> Subject: Re: [PATCH] seq_buf: let seq_buf_putmem_hex support len larger than 8 Hi Mr Steven, I found that you had ever wanted to enhance trace_seq_putmem_hex() to allow any size input(6d2289f3faa71dcc). Great minds think alike. Your enhancement will let the function more robust, I think it is very advisable. Now we only need modify two lines to solve a little flaw, and to let it more more robust. Regards, Yun On 6/25/21 12:08 PM, Steven Rostedt wrote: > [Please note: This e-mail is from an EXTERNAL e-mail address] > > On Fri, 25 Jun 2021 11:41:35 +0800 > Yun Zhou <yun.zhou@...driver.com> wrote: > >> Hi Steve, >> >> Thanks very much for your friendly and clear feedback. >> >> Although in current kernel trace_seq_putmem_hex() is only used for >> single word, >> >> I think it should/need support longer data. These are my arguments: >> >> 1. The design of double loop is used to process more data. If only >> supports single word, >> >> the inner loop is enough, and the outer loop and the following >> lines are no longer needed. >> >> len -= j / 2; >> >> hex[j++] = ' '; >> >> 2. The last line above try to split two words/dwords with space. If only >> supports single word, >> >> this strange behavior is hard to understand. >> >> 3. If it only supports single word, I think parameter 'len' is redundant. > Not really, we have to differentiate char, short, int and long long. > >> 4. The comments of both seq_buf_putmem_hex() and trace_seq_putmem_hex() >> have not >> >> indicated the scope of 'len'. >> >> 5. If it only supports single word, we need to design a new function to >> support bigger block of data. >> >> I think it is redundant since the current function can perfectly >> deal with. >> >> 6. If follow my patch, it can support any length of data, including the >> single word. >> >> How do you think? > First, since you found a real bug, we need to just fix that first (single > word as is done currently). Because this needs to go to stable, and what > you are explaining above is an enhancement, and not something that needs to > be backported. > > Second, is there a use case? Honestly, I never use the "hex" version of the > output. That was only pulled in because it was implemented in the original > code that was in the rt patch. I wish we could just get rid of it. > > Thus, if there's a use case for handling more than one word, then I'm fine > with adding that enhancement. But if it is being done just because it can > be, then I don't think we should bother. > > What use case do you have in mind? > > Anyway, please send just a fix patch, and then we can discuss the merits of > this update later. I'd like the fix to be in ASAP. > > Thanks! > > -- Steve Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.