Hi Tushar, On Mon, 2024-01-22 at 10:37 -0800, Tushar Sugandhi wrote: Missing from this and the other patch descriptions is the problem description. Please refer to the section titled "Describe your changes" in https://docs.kernel.org/process/submitting-patches.html. "Describe your problem. Whether your patch is a one-line bug fix or 5000 lines of a new feature, there must be an underlying problem that motivated you to do this work. Convince the reviewer that there is a problem worth fixing and that it makes sense for them to read past the first paragraph." In this case, "why" you need to refactor ima_dump_measurement_list() is the problem. For example: Carrying the IMA measurement list across kexec requires allocating a buffer and copying the measurement records. Separate allocating the buffer and copying the measurement records into separate functions in order to allocate the buffer at kexec "load" and copy the measurements at kexec "execute". "Once the problem is established, describe what you are actually doing about it in technical detail. It's important to describe the change in plain English for the reviewer to verify that the code is behaving as you intend it to." > Refactor ima_dump_measurement_list() to move the memory allocation part > to a separate function ima_alloc_kexec_file_buf() which allocates buffer > of size 'kexec_segment_size' at kexec 'load'. Make the local variable > ima_kexec_file in function ima_dump_measurement_list() as local static to > the file, so that it can be accessed from ima_alloc_kexec_file_buf(). > Make necessary changes to the function ima_add_kexec_buffer() to call the > above two functions. Please make this into an unordered list. -- thanks, Mimi
