Received-SPF: pass (google.com: domain of linux-kernel+bounces-106528-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Mon, 18 Mar 2024 11:45:42 -0700
From: Shradha Gupta <shradhagupta@linux.microsoft.com>
To: Easwar Hariharan <eahariha@linux.microsoft.com>
Cc: Ani Sinha <anisinha@redhat.com>, linux-kernel@vger.kernel.org,
	linux-hyperv@vger.kernel.org,
	"K. Y. Srinivasan" <kys@microsoft.com>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>,
	Long Li <longli@microsoft.com>, Olaf Hering <olaf@aepfle.de>,
	Shradha Gupta <shradhagupta@microsoft.com>
Subject: Re: [PATCH v3] hv/hv_kvp_daemon: Handle IPv4 and Ipv6 combination
 for keyfile format
Message-ID: <20240318184542.GA9011@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <1710729951-2695-1-git-send-email-shradhagupta@linux.microsoft.com>
 <9d24633d-b2bf-4cbe-86f7-6df56ba14657@linux.microsoft.com>
 <C740F1C4-CA2A-425B-8E60-0EF5C2C15270@redhat.com>
 <066c4b0c-c8a5-4abd-9456-20b0352ee1ab@linux.microsoft.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <066c4b0c-c8a5-4abd-9456-20b0352ee1ab@linux.microsoft.com>
User-Agent: Mutt/1.5.21 (2010-09-15)

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.