All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
To: Dave Young <dyoung@redhat.com>
Cc: kexec@lists.infradead.org, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org,
	Eric Biederman <ebiederm@xmission.com>
Subject: Re: [PATCH v3 3/9] kexec_file: Factor out kexec_locate_mem_hole from kexec_add_buffer.
Date: Thu, 23 Jun 2016 12:37:49 -0300	[thread overview]
Message-ID: <4923900.bfxGnX6mM2@hactar> (raw)
In-Reply-To: <2090497645.1524307.1466660647378.JavaMail.zimbra@redhat.com>

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


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
To: Dave Young <dyoung@redhat.com>
Cc: linuxppc-dev@lists.ozlabs.org, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Eric Biederman <ebiederm@xmission.com>
Subject: Re: [PATCH v3 3/9] kexec_file: Factor out kexec_locate_mem_hole from kexec_add_buffer.
Date: Thu, 23 Jun 2016 12:37:49 -0300	[thread overview]
Message-ID: <4923900.bfxGnX6mM2@hactar> (raw)
In-Reply-To: <2090497645.1524307.1466660647378.JavaMail.zimbra@redhat.com>

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

  reply	other threads:[~2016-06-23 15:38 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-21 19:48 [PATCH v3 0/9] kexec_file_load implementation for PowerPC Thiago Jung Bauermann
2016-06-21 19:48 ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 1/9] kexec_file: Remove unused members from struct kexec_buf Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 2/9] kexec_file: Generalize kexec_add_buffer Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-22 10:20   ` Dave Young
2016-06-22 10:20     ` Dave Young
2016-06-22 23:30     ` Thiago Jung Bauermann
2016-06-22 23:30       ` Thiago Jung Bauermann
2016-06-23  2:25       ` Dave Young
2016-06-23  2:25         ` Dave Young
2016-06-28 22:18         ` Thiago Jung Bauermann
2016-06-28 22:18           ` Thiago Jung Bauermann
2016-06-29 19:47           ` Dave Young
2016-06-29 19:47             ` Dave Young
2016-06-29 21:18             ` Thiago Jung Bauermann
2016-06-29 21:18               ` Thiago Jung Bauermann
2016-06-30 15:07               ` Dave Young
2016-06-30 15:07                 ` Dave Young
2016-06-30 15:49                 ` Thiago Jung Bauermann
2016-06-30 15:49                   ` Thiago Jung Bauermann
2016-06-30 16:42                   ` Thiago Jung Bauermann
2016-06-30 16:42                     ` Thiago Jung Bauermann
2016-06-30 21:43                     ` Dave Young
2016-06-30 21:43                       ` Dave Young
2016-07-01 17:51                       ` Thiago Jung Bauermann
2016-07-01 17:51                         ` Thiago Jung Bauermann
2016-07-01 18:36                         ` Dave Young
2016-07-01 18:36                           ` Dave Young
2016-07-01 20:02                           ` Thiago Jung Bauermann
2016-07-01 20:02                             ` Thiago Jung Bauermann
2016-07-01 20:31                             ` Thiago Jung Bauermann
2016-07-01 20:31                               ` Thiago Jung Bauermann
2016-07-05  0:55                               ` Dave Young
2016-07-05  0:55                                 ` Dave Young
2016-06-21 19:48 ` [PATCH v3 3/9] kexec_file: Factor out kexec_locate_mem_hole from kexec_add_buffer Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-22 10:18   ` Dave Young
2016-06-22 10:18     ` Dave Young
2016-06-22 23:34     ` Thiago Jung Bauermann
2016-06-22 23:34       ` Thiago Jung Bauermann
2016-06-23  2:30       ` Dave Young
2016-06-23  2:30         ` Dave Young
2016-06-23  5:44         ` Dave Young
2016-06-23  5:44           ` Dave Young
2016-06-23 15:37           ` Thiago Jung Bauermann [this message]
2016-06-23 15:37             ` Thiago Jung Bauermann
2016-06-27 16:19             ` Dave Young
2016-06-27 16:19               ` Dave Young
2016-06-27 16:37               ` Thiago Jung Bauermann
2016-06-27 16:37                 ` Thiago Jung Bauermann
2016-06-27 16:51                 ` Thiago Jung Bauermann
2016-06-27 16:51                   ` Thiago Jung Bauermann
2016-06-27 20:21                 ` Dave Young
2016-06-27 20:21                   ` Dave Young
2016-06-28 19:20                   ` Dave Young
2016-06-28 19:20                     ` Dave Young
2016-06-28 22:18                     ` Thiago Jung Bauermann
2016-06-28 22:18                       ` Thiago Jung Bauermann
2016-06-29 19:45                       ` Dave Young
2016-06-29 19:45                         ` Dave Young
2016-06-29 21:09                         ` Thiago Jung Bauermann
2016-06-29 21:09                           ` Thiago Jung Bauermann
2016-06-30 15:41                           ` Dave Young
2016-06-30 15:41                             ` Dave Young
2016-06-30 16:08                             ` Thiago Jung Bauermann
2016-06-30 16:08                               ` Thiago Jung Bauermann
2016-06-30 21:37                               ` Dave Young
2016-06-30 21:37                                 ` Dave Young
2016-06-21 19:48 ` [PATCH v3 4/9] powerpc: Factor out relocation code from module_64.c to elf_util_64.c Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 5/9] powerpc: Generalize elf64_apply_relocate_add Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 6/9] powerpc: Add functions to read ELF files of any endianness Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 7/9] powerpc: Implement kexec_file_load Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 8/9] powerpc: Add support for loading ELF kernels with kexec_file_load Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 9/9] powerpc: Add purgatory for kexec_file_load implementation Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-22 13:29 ` [PATCH v3 0/9] kexec_file_load implementation for PowerPC Balbir Singh
2016-06-22 13:29   ` Balbir Singh
2016-06-22 17:02   ` Thiago Jung Bauermann
2016-06-22 17:02     ` Thiago Jung Bauermann
2016-06-22 23:57     ` Balbir Singh
2016-06-22 23:57       ` Balbir Singh
2016-06-23 16:44       ` Thiago Jung Bauermann
2016-06-23 16:44         ` Thiago Jung Bauermann
2016-06-23 22:33         ` Balbir Singh
2016-06-23 22:33           ` Balbir Singh
2016-06-23 23:49           ` Thiago Jung Bauermann
2016-06-23 23:49             ` Thiago Jung Bauermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4923900.bfxGnX6mM2@hactar \
    --to=bauerman@linux.vnet.ibm.com \
    --cc=dyoung@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.