On Wed, Feb 14, 2024 at 6:43 PM Radek Krejci <radek.krejci@xxxxxxxxxx> wrote: > > Hi, > I've found a bug in modpost - when it gets the list of source files to > generate srcversion hash, it skips all the source/dependency files > except for the first one. > > There is patch [1] in v5.8rc1 replacing get_next_line() by get_line() in > parse_source_file() function. Besides other things, the difference > between those 2 functions is that get_next_line() trims the leading > spaces of the line being returned. The issue is, that the source (deps_ > located at the same directory) file names in the list, being processed > in parse_source_file(), are indented. So, when the code gets to > "Terminate line at first space, to get rid of final ' \'", it > effectively hides the source file name from further processing, since > the first space is at the beginning of the line. > > I checked this behavior with modpost from v5.4 and v5.14 (and confirmed > with the current master in git). In my case, my kernel module had just 2 > source files - mymodule.c and mymodule.h (both located at the same > directory). With modpost from v5.4, the code change in any of the files > was reflected in srcversion hash. But with modpost from v5.14 (and > master) there is no hash change when the code change appears in the > header file, which is listed at the end of the deps_ list. I believe > this is quite simple to reproduce with any module, but if needed, I can > prepare some example code to reproduce the issue. Thanks. You are right. > I noticed this [2] email thread in the list. It mentions a similar > issue. However, since it happened a half year before the change [1] was > introduced and because I was unable to find any further details, > including the promised patch, I believe that these 2 things are unrelated. Correct. They are different things. Parsing the headers located in the same directory seems to be a design. > The enclosed patch worked for me, but there might be some other > consequences that I've missed, so feel free to modify it on your own or > let me know. > > Is there anything else I can do to help fixing this issue? I think the attached patch is correct. I will pick it up later. One question is, is this feature still used? I assume the answer is yes because you noticed this bug. (but you are the first/only person who noticed it in the past 3 years) Thanks. > Regards, > Radek Krejci > > > [1] - > https://lore.kernel.org/linux-kbuild/20200601055731.3006266-26-masahiroy@xxxxxxxxxx/ > [2] - > https://lore.kernel.org/linux-kbuild/CAN19L9G-mFN-MTmw0FS3ZX4d1MjeoL2U+s-Fk7Qw9UYWn5Q1YA@xxxxxxxxxxxxxx/ -- Best Regards Masahiro Yamada
