Re: [PATCH v3] hv/hv_kvp_daemon: Handle IPv4 and Ipv6 combination for keyfile format

From: Shradha Gupta
Date: Mon Mar 18 2024 - 14:45:54 EST


On Mon, Mar 18, 2024 at 10:16:59AM -0700, Easwar Hariharan wrote:
> On 3/18/2024 10:12 AM, Ani Sinha wrote:
> >
> >
> <snip>
>
> >>> + }
> >>> +
> >>> + if (strlen(output_str)) {
> >>> + output_str[strlen(output_str) - 1] = '\0';
> >>
> >> You don't need this since you're using strncat which adds its own '\0'.
> >
> > If I understand this correctly, this code simply eliminates the extra ???,??? character in the end. Therefore it is needed.
> > Since it is not obvious, in the previous review and before, I asked the author to add a comment to explain this clearly.
> >
> >> I wasn't quite able to follow along
> >> on the discussion between Ani and you, so putting this in here in case it wasn't already mentioned.
> >>
>
> Ah, great, that makes sense. I did see that it was destroying data but didn't spend enough time to think through
> what data it was destroying, and if that was a feature or a bug. Thanks for calling it out!
>
> - Easwar
>
>
Thanks Ani, Easwar for all the comments. Apologies for not addressing some of the comments from the previous
version's review. For some reason my editor did not highlight it as expected. I will get them all with a new
version. Thanks.