On Wed, 18 Nov 2020 22:50:24 -0500 min.li...@renesas.com wrote:
> From: Min Li <min.li...@renesas.com>
>
> Feed kstrtou8 with NULL terminated string.
>
> Changes since v1:
> -Only strcpy 15 characters to leave 1 space for '\0'
>
> Signed-off-by: Min Li <min.li...@renesas.com>
> -static int idtcm_strverscmp(const char *ver1, const char *ver2)
> +static int idtcm_strverscmp(const char *version1, const char *version2)
> {
> u8 num1;
> u8 num2;
> int result = 0;
> + char ver1[16];
> + char ver2[16];
> + char *cur1;
> + char *cur2;
> + char *next1;
> + char *next2;
> +
> + strncpy(ver1, version1, 15);
> + strncpy(ver2, version2, 15);
> + cur1 = ver1;
> + cur2 = ver2;
Now there is no guarantee that the buffer is null terminated.
You need to use strscpy(ver... 16) or add ver[15] = '\0';
> /* loop through each level of the version string */
> while (result == 0) {
> + next1 = strchr(cur1, '.');
> + next2 = strchr(cur2, '.');
> +
> + /* kstrtou8 could fail for dot */
> + if (next1) {
> + *next1 = '\0';
> + next1++;
> + }
> +
> + if (next2) {
> + *next2 = '\0';
> + next2++;
> + }
> +
> /* extract leading version numbers */
> - if (kstrtou8(ver1, 10, &num1) < 0)
> + if (kstrtou8(cur1, 10, &num1) < 0)
> return -1;
>
> - if (kstrtou8(ver2, 10, &num2) < 0)
> + if (kstrtou8(cur2, 10, &num2) < 0)
> return -1;
>
> /* if numbers differ, then set the result */
> - if (num1 < num2)
> + if (num1 < num2) {
> result = -1;
Why do you set the result to something instead of just returning that
value? The kstrtou8() checks above just return value directly...
If you use a return you can save yourself all the else branches.
> - else if (num1 > num2)
> + } else if (num1 > num2) {
> result = 1;
> - else {
> + } else {
> /* if numbers are the same, go to next level */
> - ver1 = strchr(ver1, '.');
> - ver2 = strchr(ver2, '.');
> - if (!ver1 && !ver2)
> + if (!next1 && !next2)
> break;
> - else if (!ver1)
> + else if (!next1) {
> result = -1;
> - else if (!ver2)
> + } else if (!next2) {
> result = 1;
> - else {
> - ver1++;
> - ver2++;
> + } else {
> + cur1 = next1;
> + cur2 = next2;
> }
> }
> }
> +
> return result;
> }
>
