While what "tmp" points to has been cleared at the end of the first
iteration of parseNetDevLine()'s main loop, this is too late for the
first iteration's invocation of strpbrk() (copying the interface name).
Properly nul-terminate the string at population time instead, removing
the late clearing.

While there also eliminate a confusing (because of being wrong) comment:
A regex parsing error would be handled one further scope outwards. Here
we're dealing with field 1 vs any of the later fields.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Of course the function leaves much to be desired.

We had a report of a problem which may have been because of the issue
addressed here. Sadly the reporter hasn't come back with any results, so
I have to submit this largely "blindly".

--- a/tools/libs/stat/xenstat_linux.c
+++ b/tools/libs/stat/xenstat_linux.c
@@ -169,6 +169,7 @@ static int parseNetDevLine(char *line, c
 							matches[i].rm_so + 1) * sizeof(char));
 				for (x = matches[i].rm_so; x < matches[i].rm_eo; x++)
 					tmp[x - matches[i].rm_so] = line[x];
+				tmp[x - matches[i].rm_so] = 0;
 
 				/* We populate all the fields from /proc/net/dev line */
 				if (i > 1) {
@@ -225,15 +226,11 @@ static int parseNetDevLine(char *line, c
 							break;
 					}
 				}
-				else
-				/* There were errors when parsing this directly in RE. strpbrk() helps */
-				if (iface != NULL) {
+				else if (iface != NULL) {
 					char *tmp2 = strpbrk(tmp, "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789");
 					if (tmp2 != NULL)
 						strcpy(iface, tmp2);
 				}
-
-				memset(tmp, 0, matches[i].rm_eo - matches[i].rm_so);
 			}
 		}
 	}

On Wed, Jul 26, 2023 at 12:42:00PM +0200, Jan Beulich wrote:
> While what "tmp" points to has been cleared at the end of the first
> iteration of parseNetDevLine()'s main loop, this is too late for the
> first iteration's invocation of strpbrk() (copying the interface name).
> Properly nul-terminate the string at population time instead, removing
> the late clearing.
> 
> While there also eliminate a confusing (because of being wrong) comment:
> A regex parsing error would be handled one further scope outwards. Here
> we're dealing with field 1 vs any of the later fields.

I think the author of the comment meant that they couldn't create a
regex that locate the interface name, and instead used strpbrk(). But if
the comment is confusing, it's not helpful.

> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

On 26.07.23 12:42, Jan Beulich wrote:
> While what "tmp" points to has been cleared at the end of the first
> iteration of parseNetDevLine()'s main loop, this is too late for the
> first iteration's invocation of strpbrk() (copying the interface name).
> Properly nul-terminate the string at population time instead, removing
> the late clearing.
> 
> While there also eliminate a confusing (because of being wrong) comment:
> A regex parsing error would be handled one further scope outwards. Here
> we're dealing with field 1 vs any of the later fields.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen