linux-kernel - Re: [PATCH v3 3/9] kexec_file: Factor out kexec_locate_mem_hole from kexec_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <4923900.bfxGnX6mM2@hactar>
Date:	Thu, 23 Jun 2016 12:37:49 -0300
From:	Thiago Jung Bauermann <bauerman@...ux.vnet.ibm.com>
To:	Dave Young <dyoung@...hat.com>
Cc:	linuxppc-dev@...ts.ozlabs.org, kexec@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	Eric Biederman <ebiederm@...ssion.com>
Subject: Re: [PATCH v3 3/9] kexec_file: Factor out kexec_locate_mem_hole from kexec_add_buffer.

Am Donnerstag, 23 Juni 2016, 01:44:07 schrieb Dave Young:
> Hmm, hold on. For declaring a struct in a header file, comment should be
> just after each fields, like below, your format is for a function instead:
> struct pci_slot {
>         struct pci_bus *bus;            /* The bus this slot is on */
>         struct list_head list;          /* node in list of slots on this
> bus */ struct hotplug_slot *hotplug;   /* Hotplug info (migrate over
> time) */ unsigned char number;           /* PCI_SLOT(pci_dev->devfn) */
> struct kobject kobj;
> };

The comment style you mention above is not extractable documentation. The 
style I used is what is described in section "kernel-doc for structs, 
unions, enums, and typedefs" in Documentation/kernel-doc-nano-HOWTO.txt.

> BTW, what is @size? there's no size field in kexec_buf. I think it is not
> necessary to add these comment, they are easy to understand. If you really
> want, please rewrite them correctly, for example "image" description is
> wrong. It is not only for searching memory only, top_down description is
> also bad.

Sorry, I moved these comments from kexec_locate_mem_hole but forgot to 
rename the parameters to what they are called in struct kexec_buf. @size 
should have been @memsz (other fields also have wrong names, I'll fix them 
as well). The image description is correct in the context of where struct 
kexec_buf is used and explains what it will be used for in the function 
taking kexec_buf as an argument. It is not meant as a general description of 
the purpose of struct kimage. What is bad about the description of top_down?

I decided to add these comments because struct kexec_buf is now part of the 
kernel API for kexec. kernel-doc-nano-HOWTO.txt says:

> We definitely need kernel-doc formatted documentation for functions
> that are exported to loadable modules using EXPORT_SYMBOL.
> 
> We also look to provide kernel-doc formatted documentation for
> functions externally visible to other kernel files (not marked
> "static").
> 
> We also recommend providing kernel-doc formatted documentation
> for private (file "static") routines, for consistency of kernel
> source code layout.  But this is lower priority and at the
> discretion of the MAINTAINER of that kernel source file.

If you think they are not necessary or just add clutter I can leave them 
out.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center