From: David Miller Date: Mon, 14 Sep 2020 17:03:21 -0700 > From: Michael Witten > Date: Fri, 04 Sep 2020 19:40:00 -0000 > >> @@ -34,10 +34,10 @@ EXPORT_SYMBOL(of_console_options); >> int of_getintprop_default(struct device_node *np, const char *name, int def) >> { >> struct property *prop; >> - int len; >> + int size; >> >> - prop = of_find_property(np, name, &len); >> - if (!prop || len != 4) >> + prop = of_find_property(np, name, &size); >> + if (!prop || size != 4) >> return def; > > This is just changing the variable name and makes no functional change > at all, and therefore is gratuitous. > > Please only include pure functional changes that fix the bug(s) in > question. > > [...] There's a reason the variable is named "size" (or even "len") rather than: v75127e6344 A name is functional; it is the only way we have to structure a [human] reader's conceptual understanding of what's going on. The name "len" is a poor choice; it added to my uncertainty when I began trying to understand the code in question. As explained in the commit message: | String Size | =========== | | There is an important distinction to be made between the following: | | * A nul-terminated string's size | * A nul-terminated string's length | | This commit tries to make this distinction as much as possible, | and assumes that all strings are intended to be nul-terminated. | The result is the following: | | * Sometimes a variable's name is simply changed (e.g., from | 'len' to 'size'). | | * Sometimes 'strlen()' is called rather than relying on | some buffer size. | | * Sometimes, there is the replacement of code that erroneously | uses string length rather than string size. | | All together, these changes make the code more robust and correct. Are we trying to improve the code or not? Also, this name change is like a surgeon removing a benign anomaly while the abdomen is open for some other purpose; it's strategic, not "gratuitous". Sincerely, Michael Witten