| Message ID | 20230602013358.900637-5-jhubbard@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 6/5/23 08:38, Peter Xu wrote: ... >>>> I'm probably missing something, but what is the stop variable supposed to do >>>> here? It's completely unused, no? >>>> >>>> if (!strchr(end_addr, ' ')) { >>>> printf("cannot parse /proc/self/maps\n"); >>>> goto out; >>>> } >> >> Yes it is! I certainly had tunnel vision on that one. I've changed the >> patch to simply delete that line, for v2, thanks. >> >>> >>> I guess it wanted to do "*stop = '\0'" but it just didn't matter a lot >>> since the sscanf() just worked.. >>> >> >> Maybe, yes. Hard to tell the original intent at this point...it might >> have been used in an early draft version of the loop that didn't get >> posted, perhaps. > > I'm pretty sure of it.. see the pattern: > > end_addr = strchr(line, '-'); > if (!end_addr) { > printf("cannot parse /proc/self/maps\n"); > goto out; > } > *end_addr = '\0'; > > And... > > stop = strchr(end_addr, ' '); > if (!stop) { > printf("cannot parse /proc/self/maps\n"); > goto out; > } > stop = '\0'; <------------------- only diff here > Yes, and that pattern shows why it wants to be "*stop = '\0';", but it doesn't show why the author wasted a line of code in the first place, setting a variable that is not used afterwards. In other words, changing this to "*stop = '\0';" would make it look pretty, but it's a non-functional line of code to add. Which is why I think it should just be deleted at this point. thanks,
diff --git a/tools/testing/selftests/mm/mlock2-tests.c b/tools/testing/selftests/mm/mlock2-tests.c index 11b2301f3aa3..8ee95077dc25 100644 --- a/tools/testing/selftests/mm/mlock2-tests.c +++ b/tools/testing/selftests/mm/mlock2-tests.c @@ -50,7 +50,7 @@ static int get_vm_area(unsigned long addr, struct vm_boundaries *area) printf("cannot parse /proc/self/maps\n"); goto out; } - stop = '\0'; + stop = "\0"; sscanf(line, "%lx", &start); sscanf(end_addr, "%lx", &end);
The stop variable is a char*, so use "\0" when assigning to it, rather than attempting to assign a character type. This was generating a warning when compiling with clang. Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- tools/testing/selftests/mm/mlock2-tests.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)