All of lore.kernel.org
 help / color / mirror / Atom feed
* Investigating parsing error for struct/union
@ 2021-02-22 17:27 ` Aditya
  0 siblings, 0 replies; 10+ messages in thread
From: Aditya @ 2021-02-22 17:27 UTC (permalink / raw)
  To: Lukas Bulwahn, corbet; +Cc: linux-kernel-mentees, linux-doc

Hi Jonathan, Lukas

While investigating "error: Cannot parse struct or union!", I
discovered few more patterns causing this error:
1) A large part of this error(~80%) occurs due to the presence of one
or more lines(such as '#define' lines) between commented code and
struct declaration.

For e.g., in include/linux/platform_data/apds990x.h:

/**
 * struct apds990x_chip_factors - defines effect of the cover window
 * @ga: Total glass attenuation
 * @cf1: clear channel factor 1 for raw to lux conversion
 * @irf1: IR channel factor 1 for raw to lux conversion
 * @cf2: clear channel factor 2 for raw to lux conversion
 * @irf2: IR channel factor 2 for raw to lux conversion
 * @df: device factor for conversion formulas
 *
 * Structure for tuning ALS calculation to match with environment.
 * Values depend on the material above the sensor and the sensor
 * itself. If the GA is zero, driver will use uncovered sensor default
values
 * format: decimal value * APDS_PARAM_SCALE except df which is plain
integer.
 */
#define APDS_PARAM_SCALE 4096
struct apds990x_chip_factors {
	int ga;
	int cf1;
	int irf1;
	int cf2;
	int irf2;
	int df;
};


2) If struct does not contain any members, for e.g., in
include/linux/xz.h:

/**
 * struct xz_dec - Opaque type to hold the XZ decoder state
 */
struct xz_dec;

Here, it causes error as the curly braces and members expected by the
regex, are absent.
This kind of use has also been made in include/linux/zstd.h:

/**
 * struct ZSTD_DDict - a digested dictionary to be used for decompression
 */
typedef struct ZSTD_DDict_s ZSTD_DDict;


3) Different Syntax than expected. For e.g.:
    a) struct xyz struct_name {} syntax. This syntax has been used in
arch/arm/mach-omap2/omap_hwmod_common_data.c file
    b) "static __maybe_unused const struct st_sensors_platform_data
default_press_pdata = {" in drivers/iio/pressure/st_pressure.h.
This kind of syntax has also been used in
drivers/iio/accel/st_accel.h, and drivers/iio/gyro/st_gyro.h

I wanted to take your opinion if we should extend support for any of
these syntax, causing the error. If not, perhaps we can make the
documentation a bit clear, atleast for (1), which causes most of these
errors; so as to not include any lines between comment and struct
declaration.

What do you think?

Thanks
Aditya

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Linux-kernel-mentees] Investigating parsing error for struct/union
@ 2021-02-22 17:27 ` Aditya
  0 siblings, 0 replies; 10+ messages in thread
From: Aditya @ 2021-02-22 17:27 UTC (permalink / raw)
  To: Lukas Bulwahn, corbet; +Cc: linux-kernel-mentees, linux-doc

Hi Jonathan, Lukas

While investigating "error: Cannot parse struct or union!", I
discovered few more patterns causing this error:
1) A large part of this error(~80%) occurs due to the presence of one
or more lines(such as '#define' lines) between commented code and
struct declaration.

For e.g., in include/linux/platform_data/apds990x.h:

/**
 * struct apds990x_chip_factors - defines effect of the cover window
 * @ga: Total glass attenuation
 * @cf1: clear channel factor 1 for raw to lux conversion
 * @irf1: IR channel factor 1 for raw to lux conversion
 * @cf2: clear channel factor 2 for raw to lux conversion
 * @irf2: IR channel factor 2 for raw to lux conversion
 * @df: device factor for conversion formulas
 *
 * Structure for tuning ALS calculation to match with environment.
 * Values depend on the material above the sensor and the sensor
 * itself. If the GA is zero, driver will use uncovered sensor default
values
 * format: decimal value * APDS_PARAM_SCALE except df which is plain
integer.
 */
#define APDS_PARAM_SCALE 4096
struct apds990x_chip_factors {
	int ga;
	int cf1;
	int irf1;
	int cf2;
	int irf2;
	int df;
};


2) If struct does not contain any members, for e.g., in
include/linux/xz.h:

/**
 * struct xz_dec - Opaque type to hold the XZ decoder state
 */
struct xz_dec;

Here, it causes error as the curly braces and members expected by the
regex, are absent.
This kind of use has also been made in include/linux/zstd.h:

/**
 * struct ZSTD_DDict - a digested dictionary to be used for decompression
 */
typedef struct ZSTD_DDict_s ZSTD_DDict;


3) Different Syntax than expected. For e.g.:
    a) struct xyz struct_name {} syntax. This syntax has been used in
arch/arm/mach-omap2/omap_hwmod_common_data.c file
    b) "static __maybe_unused const struct st_sensors_platform_data
default_press_pdata = {" in drivers/iio/pressure/st_pressure.h.
This kind of syntax has also been used in
drivers/iio/accel/st_accel.h, and drivers/iio/gyro/st_gyro.h

I wanted to take your opinion if we should extend support for any of
these syntax, causing the error. If not, perhaps we can make the
documentation a bit clear, atleast for (1), which causes most of these
errors; so as to not include any lines between comment and struct
declaration.

What do you think?

Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Investigating parsing error for struct/union
  2021-02-22 17:27 ` [Linux-kernel-mentees] " Aditya
@ 2021-02-22 18:09   ` Lukas Bulwahn
  -1 siblings, 0 replies; 10+ messages in thread
From: Lukas Bulwahn @ 2021-02-22 18:09 UTC (permalink / raw)
  To: Aditya; +Cc: Jonathan Corbet, linux-kernel-mentees, open list:DOCUMENTATION

On Mon, Feb 22, 2021 at 6:27 PM Aditya <yashsri421@gmail.com> wrote:
>
> Hi Jonathan, Lukas
>
> While investigating "error: Cannot parse struct or union!", I
> discovered few more patterns causing this error:
> 1) A large part of this error(~80%) occurs due to the presence of one
> or more lines(such as '#define' lines) between commented code and
> struct declaration.
>
> For e.g., in include/linux/platform_data/apds990x.h:
>
> /**
>  * struct apds990x_chip_factors - defines effect of the cover window
>  * @ga: Total glass attenuation
>  * @cf1: clear channel factor 1 for raw to lux conversion
>  * @irf1: IR channel factor 1 for raw to lux conversion
>  * @cf2: clear channel factor 2 for raw to lux conversion
>  * @irf2: IR channel factor 2 for raw to lux conversion
>  * @df: device factor for conversion formulas
>  *
>  * Structure for tuning ALS calculation to match with environment.
>  * Values depend on the material above the sensor and the sensor
>  * itself. If the GA is zero, driver will use uncovered sensor default
> values
>  * format: decimal value * APDS_PARAM_SCALE except df which is plain
> integer.
>  */
> #define APDS_PARAM_SCALE 4096
> struct apds990x_chip_factors {
>         int ga;
>         int cf1;
>         int irf1;
>         int cf2;
>         int irf2;
>         int df;
> };
>
>

Aditya, I independently made a similar observation and noted this
issue in my personal notes, which will serve for a cleanup patch
series, as follows:

"SKIP DEFINITIONS" Feature:

The kernel-doc should proceed the code, and the required definition of
data structures should stay close to the function in between the
kernel-doc comment and the function definition.

Let kernel-doc know to skip certain definitions; the actual definition
documented will follow directly after the skipped definitions.

E.g.,

include/linux/cgroup.h:450: task_css_set_check: SKIP DEFINITIONS
(struct mutex cgroup_mutex; spinlock_t css_set_lock)
include/linux/hid-sensor-hub.h:174:
sensor_hub_input_attr_get_raw_value: SKIP DEFINITIONS (enum
sensor_hub_read_flags)

I have not thought about a good syntax for that:

maybe something like:
/**
* foo - description
*
* @arg: description
*
* ::skip: struct bar, enum blub, define MACRO
*/

where "::" serves as some annotation to instruct kernel-doc

> 2) If struct does not contain any members, for e.g., in
> include/linux/xz.h:
>
> /**
>  * struct xz_dec - Opaque type to hold the XZ decoder state
>  */
> struct xz_dec;
>
> Here, it causes error as the curly braces and members expected by the
> regex, are absent.
> This kind of use has also been made in include/linux/zstd.h:
>
> /**
>  * struct ZSTD_DDict - a digested dictionary to be used for decompression
>  */
> typedef struct ZSTD_DDict_s ZSTD_DDict;
>
>
> 3) Different Syntax than expected. For e.g.:
>     a) struct xyz struct_name {} syntax. This syntax has been used in
> arch/arm/mach-omap2/omap_hwmod_common_data.c file
>     b) "static __maybe_unused const struct st_sensors_platform_data
> default_press_pdata = {" in drivers/iio/pressure/st_pressure.h.
> This kind of syntax has also been used in
> drivers/iio/accel/st_accel.h, and drivers/iio/gyro/st_gyro.h
>
> I wanted to take your opinion if we should extend support for any of
> these syntax, causing the error. If not, perhaps we can make the
> documentation a bit clear, atleast for (1), which causes most of these
> errors; so as to not include any lines between comment and struct
> declaration.
>
> What do you think?
>

I certainly appreciate it if you could work on that SKIP_DEFINITION
feature, once we agreed on some first syntax for this feature.

Lukas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] Investigating parsing error for struct/union
@ 2021-02-22 18:09   ` Lukas Bulwahn
  0 siblings, 0 replies; 10+ messages in thread
From: Lukas Bulwahn @ 2021-02-22 18:09 UTC (permalink / raw)
  To: Aditya; +Cc: open list:DOCUMENTATION, linux-kernel-mentees, Jonathan Corbet

On Mon, Feb 22, 2021 at 6:27 PM Aditya <yashsri421@gmail.com> wrote:
>
> Hi Jonathan, Lukas
>
> While investigating "error: Cannot parse struct or union!", I
> discovered few more patterns causing this error:
> 1) A large part of this error(~80%) occurs due to the presence of one
> or more lines(such as '#define' lines) between commented code and
> struct declaration.
>
> For e.g., in include/linux/platform_data/apds990x.h:
>
> /**
>  * struct apds990x_chip_factors - defines effect of the cover window
>  * @ga: Total glass attenuation
>  * @cf1: clear channel factor 1 for raw to lux conversion
>  * @irf1: IR channel factor 1 for raw to lux conversion
>  * @cf2: clear channel factor 2 for raw to lux conversion
>  * @irf2: IR channel factor 2 for raw to lux conversion
>  * @df: device factor for conversion formulas
>  *
>  * Structure for tuning ALS calculation to match with environment.
>  * Values depend on the material above the sensor and the sensor
>  * itself. If the GA is zero, driver will use uncovered sensor default
> values
>  * format: decimal value * APDS_PARAM_SCALE except df which is plain
> integer.
>  */
> #define APDS_PARAM_SCALE 4096
> struct apds990x_chip_factors {
>         int ga;
>         int cf1;
>         int irf1;
>         int cf2;
>         int irf2;
>         int df;
> };
>
>

Aditya, I independently made a similar observation and noted this
issue in my personal notes, which will serve for a cleanup patch
series, as follows:

"SKIP DEFINITIONS" Feature:

The kernel-doc should proceed the code, and the required definition of
data structures should stay close to the function in between the
kernel-doc comment and the function definition.

Let kernel-doc know to skip certain definitions; the actual definition
documented will follow directly after the skipped definitions.

E.g.,

include/linux/cgroup.h:450: task_css_set_check: SKIP DEFINITIONS
(struct mutex cgroup_mutex; spinlock_t css_set_lock)
include/linux/hid-sensor-hub.h:174:
sensor_hub_input_attr_get_raw_value: SKIP DEFINITIONS (enum
sensor_hub_read_flags)

I have not thought about a good syntax for that:

maybe something like:
/**
* foo - description
*
* @arg: description
*
* ::skip: struct bar, enum blub, define MACRO
*/

where "::" serves as some annotation to instruct kernel-doc

> 2) If struct does not contain any members, for e.g., in
> include/linux/xz.h:
>
> /**
>  * struct xz_dec - Opaque type to hold the XZ decoder state
>  */
> struct xz_dec;
>
> Here, it causes error as the curly braces and members expected by the
> regex, are absent.
> This kind of use has also been made in include/linux/zstd.h:
>
> /**
>  * struct ZSTD_DDict - a digested dictionary to be used for decompression
>  */
> typedef struct ZSTD_DDict_s ZSTD_DDict;
>
>
> 3) Different Syntax than expected. For e.g.:
>     a) struct xyz struct_name {} syntax. This syntax has been used in
> arch/arm/mach-omap2/omap_hwmod_common_data.c file
>     b) "static __maybe_unused const struct st_sensors_platform_data
> default_press_pdata = {" in drivers/iio/pressure/st_pressure.h.
> This kind of syntax has also been used in
> drivers/iio/accel/st_accel.h, and drivers/iio/gyro/st_gyro.h
>
> I wanted to take your opinion if we should extend support for any of
> these syntax, causing the error. If not, perhaps we can make the
> documentation a bit clear, atleast for (1), which causes most of these
> errors; so as to not include any lines between comment and struct
> declaration.
>
> What do you think?
>

I certainly appreciate it if you could work on that SKIP_DEFINITION
feature, once we agreed on some first syntax for this feature.

Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Investigating parsing error for struct/union
  2021-02-22 17:27 ` [Linux-kernel-mentees] " Aditya
@ 2021-02-22 21:37   ` Randy Dunlap
  -1 siblings, 0 replies; 10+ messages in thread
From: Randy Dunlap @ 2021-02-22 21:37 UTC (permalink / raw)
  To: Aditya, Lukas Bulwahn, corbet; +Cc: linux-kernel-mentees, linux-doc

On 2/22/21 9:27 AM, Aditya wrote:
> Hi Jonathan, Lukas
> 
> While investigating "error: Cannot parse struct or union!", I
> discovered few more patterns causing this error:
> 1) A large part of this error(~80%) occurs due to the presence of one
> or more lines(such as '#define' lines) between commented code and
> struct declaration.
> 
> For e.g., in include/linux/platform_data/apds990x.h:
> 
> /**
>  * struct apds990x_chip_factors - defines effect of the cover window
>  * @ga: Total glass attenuation
>  * @cf1: clear channel factor 1 for raw to lux conversion
>  * @irf1: IR channel factor 1 for raw to lux conversion
>  * @cf2: clear channel factor 2 for raw to lux conversion
>  * @irf2: IR channel factor 2 for raw to lux conversion
>  * @df: device factor for conversion formulas
>  *
>  * Structure for tuning ALS calculation to match with environment.
>  * Values depend on the material above the sensor and the sensor
>  * itself. If the GA is zero, driver will use uncovered sensor default
> values
>  * format: decimal value * APDS_PARAM_SCALE except df which is plain
> integer.
>  */
> #define APDS_PARAM_SCALE 4096
> struct apds990x_chip_factors {
> 	int ga;
> 	int cf1;
> 	int irf1;
> 	int cf2;
> 	int irf2;
> 	int df;
> };

I have been known to move a few of macros such as this one.
OTOH, if you can coax kernel-doc to skip macros and find a
struct/union/function, that would be OK IMO.

> 
> 
> 2) If struct does not contain any members, for e.g., in
> include/linux/xz.h:
> 
> /**
>  * struct xz_dec - Opaque type to hold the XZ decoder state
>  */
> struct xz_dec;
> 
> Here, it causes error as the curly braces and members expected by the
> regex, are absent.

I.e., there is no kernel-doc description there at all.

Then in lib/xz/xz_dec_stream.c, the struct is defined with no
kernel-doc at all.

IMO the struct in lib/xz/xz_dec_stream.c should be marked as
kernel-doc and then all of the fields in it marked as
  /* private: */
since the author(s) seem to want this struct to be private/opaque.

> This kind of use has also been made in include/linux/zstd.h:
> 
> /**
>  * struct ZSTD_DDict - a digested dictionary to be used for decompression
>  */
> typedef struct ZSTD_DDict_s ZSTD_DDict;
> 
> 
> 3) Different Syntax than expected. For e.g.:
>     a) struct xyz struct_name {} syntax. This syntax has been used in
> arch/arm/mach-omap2/omap_hwmod_common_data.c file
>     b) "static __maybe_unused const struct st_sensors_platform_data
> default_press_pdata = {" in drivers/iio/pressure/st_pressure.h.
> This kind of syntax has also been used in
> drivers/iio/accel/st_accel.h, and drivers/iio/gyro/st_gyro.h

kernel-doc should just ignore/skip/drop __maybe_unused.

> 
> I wanted to take your opinion if we should extend support for any of
> these syntax, causing the error. If not, perhaps we can make the
> documentation a bit clear, atleast for (1), which causes most of these
> errors; so as to not include any lines between comment and struct
> declaration.
> 
> What do you think?


-- 
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] Investigating parsing error for struct/union
@ 2021-02-22 21:37   ` Randy Dunlap
  0 siblings, 0 replies; 10+ messages in thread
From: Randy Dunlap @ 2021-02-22 21:37 UTC (permalink / raw)
  To: Aditya, Lukas Bulwahn, corbet; +Cc: linux-kernel-mentees, linux-doc

On 2/22/21 9:27 AM, Aditya wrote:
> Hi Jonathan, Lukas
> 
> While investigating "error: Cannot parse struct or union!", I
> discovered few more patterns causing this error:
> 1) A large part of this error(~80%) occurs due to the presence of one
> or more lines(such as '#define' lines) between commented code and
> struct declaration.
> 
> For e.g., in include/linux/platform_data/apds990x.h:
> 
> /**
>  * struct apds990x_chip_factors - defines effect of the cover window
>  * @ga: Total glass attenuation
>  * @cf1: clear channel factor 1 for raw to lux conversion
>  * @irf1: IR channel factor 1 for raw to lux conversion
>  * @cf2: clear channel factor 2 for raw to lux conversion
>  * @irf2: IR channel factor 2 for raw to lux conversion
>  * @df: device factor for conversion formulas
>  *
>  * Structure for tuning ALS calculation to match with environment.
>  * Values depend on the material above the sensor and the sensor
>  * itself. If the GA is zero, driver will use uncovered sensor default
> values
>  * format: decimal value * APDS_PARAM_SCALE except df which is plain
> integer.
>  */
> #define APDS_PARAM_SCALE 4096
> struct apds990x_chip_factors {
> 	int ga;
> 	int cf1;
> 	int irf1;
> 	int cf2;
> 	int irf2;
> 	int df;
> };

I have been known to move a few of macros such as this one.
OTOH, if you can coax kernel-doc to skip macros and find a
struct/union/function, that would be OK IMO.

> 
> 
> 2) If struct does not contain any members, for e.g., in
> include/linux/xz.h:
> 
> /**
>  * struct xz_dec - Opaque type to hold the XZ decoder state
>  */
> struct xz_dec;
> 
> Here, it causes error as the curly braces and members expected by the
> regex, are absent.

I.e., there is no kernel-doc description there at all.

Then in lib/xz/xz_dec_stream.c, the struct is defined with no
kernel-doc at all.

IMO the struct in lib/xz/xz_dec_stream.c should be marked as
kernel-doc and then all of the fields in it marked as
  /* private: */
since the author(s) seem to want this struct to be private/opaque.

> This kind of use has also been made in include/linux/zstd.h:
> 
> /**
>  * struct ZSTD_DDict - a digested dictionary to be used for decompression
>  */
> typedef struct ZSTD_DDict_s ZSTD_DDict;
> 
> 
> 3) Different Syntax than expected. For e.g.:
>     a) struct xyz struct_name {} syntax. This syntax has been used in
> arch/arm/mach-omap2/omap_hwmod_common_data.c file
>     b) "static __maybe_unused const struct st_sensors_platform_data
> default_press_pdata = {" in drivers/iio/pressure/st_pressure.h.
> This kind of syntax has also been used in
> drivers/iio/accel/st_accel.h, and drivers/iio/gyro/st_gyro.h

kernel-doc should just ignore/skip/drop __maybe_unused.

> 
> I wanted to take your opinion if we should extend support for any of
> these syntax, causing the error. If not, perhaps we can make the
> documentation a bit clear, atleast for (1), which causes most of these
> errors; so as to not include any lines between comment and struct
> declaration.
> 
> What do you think?


-- 
~Randy

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Investigating parsing error for struct/union
  2021-02-22 18:09   ` [Linux-kernel-mentees] " Lukas Bulwahn
@ 2021-02-22 21:49     ` Jonathan Corbet
  -1 siblings, 0 replies; 10+ messages in thread
From: Jonathan Corbet @ 2021-02-22 21:49 UTC (permalink / raw)
  To: Lukas Bulwahn, Aditya; +Cc: linux-kernel-mentees, open list:DOCUMENTATION

Lukas Bulwahn <lukas.bulwahn@gmail.com> writes:

> Aditya, I independently made a similar observation and noted this
> issue in my personal notes, which will serve for a cleanup patch
> series, as follows:
>
> "SKIP DEFINITIONS" Feature:
>
> The kernel-doc should proceed the code, and the required definition of
> data structures should stay close to the function in between the
> kernel-doc comment and the function definition.
>
> Let kernel-doc know to skip certain definitions; the actual definition
> documented will follow directly after the skipped definitions.
>
> E.g.,
>
> include/linux/cgroup.h:450: task_css_set_check: SKIP DEFINITIONS
> (struct mutex cgroup_mutex; spinlock_t css_set_lock)
> include/linux/hid-sensor-hub.h:174:
> sensor_hub_input_attr_get_raw_value: SKIP DEFINITIONS (enum
> sensor_hub_read_flags)
>
> I have not thought about a good syntax for that:
>
> maybe something like:
> /**
> * foo - description
> *
> * @arg: description
> *
> * ::skip: struct bar, enum blub, define MACRO
> */
>
> where "::" serves as some annotation to instruct kernel-doc

Honestly, we will never get that sort of annotation into the source
without encountering substantial pushback, and it will also be hard to
keep them up to date.  I don't see that as being the right path forward.

The better solution, for now, would be to just insist that the kerneldoc
comments be next to the thing they are describing, and to submit patches
fixing these issues to the appropriate maintainers.  Mauro and others
have been doing a fair amount of that already.

It might also be nice to make scripts/kernel-doc smarter so that it can
handle any degree of separation between the comment and the
declaration.  But that would complicate an already gnarly script for
(IMO) relatively little gain.

Thanks,

jon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] Investigating parsing error for struct/union
@ 2021-02-22 21:49     ` Jonathan Corbet
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Corbet @ 2021-02-22 21:49 UTC (permalink / raw)
  To: Lukas Bulwahn, Aditya; +Cc: linux-kernel-mentees, open list:DOCUMENTATION

Lukas Bulwahn <lukas.bulwahn@gmail.com> writes:

> Aditya, I independently made a similar observation and noted this
> issue in my personal notes, which will serve for a cleanup patch
> series, as follows:
>
> "SKIP DEFINITIONS" Feature:
>
> The kernel-doc should proceed the code, and the required definition of
> data structures should stay close to the function in between the
> kernel-doc comment and the function definition.
>
> Let kernel-doc know to skip certain definitions; the actual definition
> documented will follow directly after the skipped definitions.
>
> E.g.,
>
> include/linux/cgroup.h:450: task_css_set_check: SKIP DEFINITIONS
> (struct mutex cgroup_mutex; spinlock_t css_set_lock)
> include/linux/hid-sensor-hub.h:174:
> sensor_hub_input_attr_get_raw_value: SKIP DEFINITIONS (enum
> sensor_hub_read_flags)
>
> I have not thought about a good syntax for that:
>
> maybe something like:
> /**
> * foo - description
> *
> * @arg: description
> *
> * ::skip: struct bar, enum blub, define MACRO
> */
>
> where "::" serves as some annotation to instruct kernel-doc

Honestly, we will never get that sort of annotation into the source
without encountering substantial pushback, and it will also be hard to
keep them up to date.  I don't see that as being the right path forward.

The better solution, for now, would be to just insist that the kerneldoc
comments be next to the thing they are describing, and to submit patches
fixing these issues to the appropriate maintainers.  Mauro and others
have been doing a fair amount of that already.

It might also be nice to make scripts/kernel-doc smarter so that it can
handle any degree of separation between the comment and the
declaration.  But that would complicate an already gnarly script for
(IMO) relatively little gain.

Thanks,

jon
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Investigating parsing error for struct/union
  2021-02-22 17:27 ` [Linux-kernel-mentees] " Aditya
@ 2021-02-22 21:56   ` Jonathan Corbet
  -1 siblings, 0 replies; 10+ messages in thread
From: Jonathan Corbet @ 2021-02-22 21:56 UTC (permalink / raw)
  To: Aditya, Lukas Bulwahn; +Cc: linux-kernel-mentees, linux-doc

Aditya <yashsri421@gmail.com> writes:

> 1) A large part of this error(~80%) occurs due to the presence of one
> or more lines(such as '#define' lines) between commented code and
> struct declaration.

See my other email for this one.

> 2) If struct does not contain any members, for e.g., in
> include/linux/xz.h:
>
> /**
>  * struct xz_dec - Opaque type to hold the XZ decoder state
>  */
> struct xz_dec;
>
> Here, it causes error as the curly braces and members expected by the
> regex, are absent.

Here, too, the real problem is that the kerneldoc comment is in the
wrong place.  There will be a real declaration for that structure
somewhere; in this case it's lib/xz/xz_dec_stream.c.  *That* is where
the kerneldoc comment should be.

That said, the above isn't a kerneldoc comment anyway; it doesn't really
describe anything useful and would generate lots of warnings if it were
in the right place.  The right solution is either (1) write a proper
comment describing this structure, or (2) delete the extra "*" at the
beginning of this comment.

> 3) Different Syntax than expected. For e.g.:
>     a) struct xyz struct_name {} syntax. This syntax has been used in
> arch/arm/mach-omap2/omap_hwmod_common_data.c file

*Please* give an actual example, from the source, of what you are
talking about for cases like this; I spent a while looking at that file
trying to figure out what you were talking about.  I assume you're
referring to silliness like this:

/**
 * struct omap_hwmod_sysc_type2 - TYPE2 sysconfig scheme.
 *
 * To be used by hwmod structure to specify the sysconfig offsets if the
 * device ip is compliant with the new PRCM protocol defined for new
 * OMAP4 IPs.
 */
struct sysc_regbits omap_hwmod_sysc_type2 = {
	.midle_shift	= SYSC_TYPE2_MIDLEMODE_SHIFT,
	.sidle_shift	= SYSC_TYPE2_SIDLEMODE_SHIFT,
	.srst_shift	= SYSC_TYPE2_SOFTRESET_SHIFT,
	.dmadisable_shift = SYSC_TYPE2_DMADISABLE_SHIFT,
};

...right?  Again, the problem here is that these are not proper
kerneldoc comments.  We're not describing the structure, this is just a
comment describing a specific variable.  The right fix, once again, is
s%/**%/*%

>     b) "static __maybe_unused const struct st_sensors_platform_data
> default_press_pdata = {" in drivers/iio/pressure/st_pressure.h.
> This kind of syntax has also been used in
> drivers/iio/accel/st_accel.h, and drivers/iio/gyro/st_gyro.h

Same problem here; the fix is not in the docs build system.

Thanks,

jon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Linux-kernel-mentees] Investigating parsing error for struct/union
@ 2021-02-22 21:56   ` Jonathan Corbet
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Corbet @ 2021-02-22 21:56 UTC (permalink / raw)
  To: Aditya, Lukas Bulwahn; +Cc: linux-kernel-mentees, linux-doc

Aditya <yashsri421@gmail.com> writes:

> 1) A large part of this error(~80%) occurs due to the presence of one
> or more lines(such as '#define' lines) between commented code and
> struct declaration.

See my other email for this one.

> 2) If struct does not contain any members, for e.g., in
> include/linux/xz.h:
>
> /**
>  * struct xz_dec - Opaque type to hold the XZ decoder state
>  */
> struct xz_dec;
>
> Here, it causes error as the curly braces and members expected by the
> regex, are absent.

Here, too, the real problem is that the kerneldoc comment is in the
wrong place.  There will be a real declaration for that structure
somewhere; in this case it's lib/xz/xz_dec_stream.c.  *That* is where
the kerneldoc comment should be.

That said, the above isn't a kerneldoc comment anyway; it doesn't really
describe anything useful and would generate lots of warnings if it were
in the right place.  The right solution is either (1) write a proper
comment describing this structure, or (2) delete the extra "*" at the
beginning of this comment.

> 3) Different Syntax than expected. For e.g.:
>     a) struct xyz struct_name {} syntax. This syntax has been used in
> arch/arm/mach-omap2/omap_hwmod_common_data.c file

*Please* give an actual example, from the source, of what you are
talking about for cases like this; I spent a while looking at that file
trying to figure out what you were talking about.  I assume you're
referring to silliness like this:

/**
 * struct omap_hwmod_sysc_type2 - TYPE2 sysconfig scheme.
 *
 * To be used by hwmod structure to specify the sysconfig offsets if the
 * device ip is compliant with the new PRCM protocol defined for new
 * OMAP4 IPs.
 */
struct sysc_regbits omap_hwmod_sysc_type2 = {
	.midle_shift	= SYSC_TYPE2_MIDLEMODE_SHIFT,
	.sidle_shift	= SYSC_TYPE2_SIDLEMODE_SHIFT,
	.srst_shift	= SYSC_TYPE2_SOFTRESET_SHIFT,
	.dmadisable_shift = SYSC_TYPE2_DMADISABLE_SHIFT,
};

...right?  Again, the problem here is that these are not proper
kerneldoc comments.  We're not describing the structure, this is just a
comment describing a specific variable.  The right fix, once again, is
s%/**%/*%

>     b) "static __maybe_unused const struct st_sensors_platform_data
> default_press_pdata = {" in drivers/iio/pressure/st_pressure.h.
> This kind of syntax has also been used in
> drivers/iio/accel/st_accel.h, and drivers/iio/gyro/st_gyro.h

Same problem here; the fix is not in the docs build system.

Thanks,

jon
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-02-22 21:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-22 17:27 Investigating parsing error for struct/union Aditya
2021-02-22 17:27 ` [Linux-kernel-mentees] " Aditya
2021-02-22 18:09 ` Lukas Bulwahn
2021-02-22 18:09   ` [Linux-kernel-mentees] " Lukas Bulwahn
2021-02-22 21:49   ` Jonathan Corbet
2021-02-22 21:49     ` [Linux-kernel-mentees] " Jonathan Corbet
2021-02-22 21:37 ` Randy Dunlap
2021-02-22 21:37   ` [Linux-kernel-mentees] " Randy Dunlap
2021-02-22 21:56 ` Jonathan Corbet
2021-02-22 21:56   ` [Linux-kernel-mentees] " Jonathan Corbet

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.