Re: [PATCH v2] tap: Update some function comments for accuracy

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 16 Oct 2025 11:47:22 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Oct 16, 2025 at 01:46:24AM +0200, Stefano Brivio wrote:
> > On Mon, 13 Oct 2025 10:49:41 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > Several of the tap_push_*() functions have doc comments claiming they take
> > > the context pointer, but don't.  Some (tap_push_uh[46]) were broken fairly
> > > recently, but others (tap_push_ip[46]h) have been broken for a long time.
> > > 
> > > Regardless, fix all the doc comments.
> > > 
> > > Reported-by: Stefano Brivio <sbrivio@redhat.com>
> > > Fixes: 82a839be9 ("tap: break out building of udp header from tap_udp4_send function")
> > > Fixes: 87e6a4644 ("tap: break out building of udp header from tap_udp6_send function")
> > > Fixes: 2dbc622f5 ("tap: Split tap_ip4_send() into UDP and ICMP variants")
> > > Fixes: 9d8dd8b6f ("tap: Split tap_ip6_send() into UDP and ICMP variants")
> > > 
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>  
> > 
> > Applied, with one minor change to tags (I plan to add some stuff,
> > including this bit, to the new CONTRIBUTING.md).
> > 
> > For consistency with the Linux kernel, we use SHAs abbreviated to 12
> > digits there, even though 9-digit abbreviations (produced by default
> > git-publish settings, I guess)  
> 
> No, git-publish doesn't manage Fixes tags.  It is the default for git
> blame, though, which is where I would have copied them from
> 
> > are unlikely to ever lead to any
> > conflict for us.
> > 
> > Well, that's true at least until the day you all discover
> > https://github.com/not-an-aardvark/lucky-commit, but at that point the
> > number of digits wouldn't make a difference.
> > 
> > So, to keep the consistency consistent, I changed those to 12-digit
> > forms and dropped the extra newline (also added by git-publish). If
> > you fancy a script checking that for you, see:
> > 
> >   https://lore.kernel.org/all/20190220213729.49deb54f@redhat.com/  
> 
> Thanks, I've put that into my local hooks.

Thanks. By the way, there's still the occasional blank line before
Signed-off-tag: in your patches, which I'm fairly sure comes from
git-publish.

Maybe it would be good to propose a patch for some kind of "Linux
kernel mode" for git-publish omitting that extra line, and probably a
couple of other tweaks? I don't remember what the other differences
(compared to QEMU) were.

It takes me less than a second to drop that line on merge though, so
it's definitely not an issue.

-- 
Stefano