DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Marat Khalili <marat.khalili@huawei.com>
To: Marat Khalili <marat.khalili@huawei.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"bruce.richardson@intel.com" <bruce.richardson@intel.com>
Subject: RE: [PATCH v3 0/8] doc: build for all public headers
Date: Tue, 7 Jul 2026 11:23:31 +0000	[thread overview]
Message-ID: <b071c236c8c74b7faecf8efd4da1f46b@huawei.com> (raw)
In-Reply-To: <20260707085048.15295-1-marat.khalili@huawei.com>

Will answer the AI code review comments here.

> ## Patch 1/8: doc: detect ignored public headers
> 
> ### Errors
> 
> **Error: Resource leak in meson.build `run_command()` usage**
> The `run_command()` calls capture stdout into variables (`doc_built_dirs`, `doc_indexed_headers`) but
> there is no error handling for the command execution itself beyond `check: true`.
> If the `sed` command fails partway through (e.g., file read error, permission issue), the script
> continues with potentially incomplete data.
> While `check: true` will cause meson to fail, the failure mode is not graceful.

Requirement for the setup to survive unreadable source files is clearly excessive.
And meson reports specific command that failed quite well already.

> ### Warnings
> 
> **Warning: Hardcoded meson version check without rationale**
> Line `check_docs = is_linux and meson.version().version_compare('>= 0.60.0')` introduces a version
> check for meson 0.60.0 without explanation.
> 
> **Suggested fix:** Add a comment explaining why meson 0.60.0 is required (likely for `fs.name()` or
> another feature used in the check).

Meson prints source lines and versions required during the setup already.
Not sure who would ever benefit from this comment.

> **Warning: `run_command()` assumes specific sed implementation**
> The `sed` commands use `--regexp-extended` which is a GNU sed extension.
> On systems with BSD sed (macOS, FreeBSD), this will fail.

The check is explicitly limited to Linux.

> **Warning: Global variables `doc_built_dirs` and `doc_indexed_headers` lack context**
> These variables are set in the top-level `meson.build` but used in `drivers/meson.build` and
> `lib/meson.build` without clear documentation of their contract.

The names are self-descriptive IMO.

> ## Patch 3/8: doc: fix typos in rte_avp_common.h
> 
> ### Warnings
> 
> **Warning: Missing closing brace on line 271**
> Line 271 adds `@{` for "Access AVP device version values" but there is no corresponding `@}` to close
> this group.
> All three macros that follow (`RTE_AVP_GET_RELEASE_VERSION`, etc.) will be grouped, but the group is
> never closed.

Grouping commands are imbalanced in the file now, so the patch has to have them
imbalanced for the result to be balanced. Instead of the patch the AI reviewer
should have checked the resulting file or the fact that it builds.

> ## Patch 5/8: doc: fix typos in rte_bus_vmbus.h
> 
> ### Warnings
> 
> **Warning: Incomplete parameter documentation in `rte_vmbus_chan_send_sglist()`**
> The function takes a `flags` parameter (visible in the function signature in the actual driver code),
> but the patch removes `@param flags` from the documentation without verifying that the parameter does
> not exist.

Not sure how the model checked the function signature, but the parameter does not exist.
Which is why documentation failed to build before and builds now.

> **Warning: Parameter name mismatch in `rte_vmbus_chan_recv_raw()`**
> Documentation refers to `@param channel` but the actual parameter name is `chan` (as used in the
> corrected version).
> This is fixed by the patch, but worth noting for consistency checks.

The model notes the discrepancy that the patch already fixes.

> ## Patch 8/8: doc: add missing headers to doxy-api-index.md
> 
> ### Warnings
> 
> **Warning: BPF list formatting inconsistency**
> Lines 219-223 introduce a nested bullet list for BPF headers:
> ```markdown
>   [member](@ref rte_member.h)
>   * BPF:
>     [load and execute](@ref rte_bpf.h),
>     ...
> ```
> This creates a sub-list with only one parent item.

Nested bullet lists are used in other places in the file and even in the same patch,
seem to be a good practice actually. Not sure what "only one parent item" means.

> **Warning: Comma inconsistency in "containers" section**
> Line 231 adds `[member](@ref rte_member.h)` followed immediately by a new bullet for BPF, but previous
> list items in the same block end with commas.

Customary in the file last item in the list does not end with a comma.
The rendering was verified in the web browser.

> ---
> 
> ## Summary by Patch
> 
> | Patch | Errors | Warnings | Info |
> |-------|--------|----------|------|
> | 1/8   | 0      | 3        | 0    |
> | 2/8   | 0      | 0        | 1    |
> | 3/8   | 0      | 1        | 0    |
> | 4/8   | 0      | 0        | 0    |
> | 5/8   | 0      | 2        | 0    |
> | 6/8   | 0      | 0        | 0    |
> | 7/8   | 0      | 0        | 1    |
> | 8/8   | 0      | 2        | 0    |

Interestingly, the one found Error did not make it to the table.

This error and all 8 warnings are addressed above and not the actual problems.

      parent reply	other threads:[~2026-07-07 11:23 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30  9:03 [RFC] doc: updating doxy-api-index.md and doxy-api.conf.in Marat Khalili
2026-06-30 16:10 ` Thomas Monjalon
2026-06-30 16:45   ` Bruce Richardson
2026-07-02 20:00 ` [PATCH 0/7] doc: build for all public headers Marat Khalili
2026-07-02 20:00   ` [PATCH 1/7] doc: detect ignored " Marat Khalili
2026-07-02 20:00   ` [PATCH 2/7] doc: document rte_os.h Marat Khalili
2026-07-02 20:00   ` [PATCH 3/7] doc: fix typos in rte_bus_pci.h Marat Khalili
2026-07-02 20:00   ` [PATCH 4/7] doc: fix typos in rte_bus_vmbus.h Marat Khalili
2026-07-02 20:00   ` [PATCH 5/7] doc: add missing globs to doxy-api.conf.in Marat Khalili
2026-07-02 20:00   ` [PATCH 6/7] doc: add missing drivers " Marat Khalili
2026-07-02 20:00   ` [PATCH 7/7] doc: add missing headers to doxy-api-index.md Marat Khalili
2026-07-06 10:52   ` [PATCH v2 0/7] doc: build for all public headers Marat Khalili
2026-07-06 10:52     ` [PATCH v2 1/7] doc: detect ignored " Marat Khalili
2026-07-06 10:52     ` [PATCH v2 2/7] doc: document rte_os.h Marat Khalili
2026-07-06 10:52     ` [PATCH v2 3/7] doc: fix typos in rte_bus_pci.h Marat Khalili
2026-07-06 10:52     ` [PATCH v2 4/7] doc: fix typos in rte_bus_vmbus.h Marat Khalili
2026-07-06 10:52     ` [PATCH v2 5/7] doc: add missing globs to doxy-api.conf.in Marat Khalili
2026-07-06 10:52     ` [PATCH v2 6/7] doc: add missing drivers " Marat Khalili
2026-07-06 10:52     ` [PATCH v2 7/7] doc: add missing headers to doxy-api-index.md Marat Khalili
2026-07-06 13:14     ` [PATCH v2 0/7] doc: build for all public headers Marat Khalili
2026-07-07  8:50     ` [PATCH v3 0/8] " Marat Khalili
2026-07-07  8:50       ` [PATCH v3 1/8] doc: detect ignored " Marat Khalili
2026-07-07  8:50       ` [PATCH v3 2/8] doc: document rte_os.h Marat Khalili
2026-07-07  8:50       ` [PATCH v3 3/8] doc: fix typos in rte_avp_common.h Marat Khalili
2026-07-07  8:50       ` [PATCH v3 4/8] doc: fix typos in rte_bus_pci.h Marat Khalili
2026-07-07  8:50       ` [PATCH v3 5/8] doc: fix typos in rte_bus_vmbus.h Marat Khalili
2026-07-07  8:50       ` [PATCH v3 6/8] doc: add missing globs to doxy-api.conf.in Marat Khalili
2026-07-07  8:50       ` [PATCH v3 7/8] doc: add missing drivers " Marat Khalili
2026-07-07  8:50       ` [PATCH v3 8/8] doc: add missing headers to doxy-api-index.md Marat Khalili
2026-07-07 11:23       ` Marat Khalili [this message]

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=b071c236c8c74b7faecf8efd4da1f46b@huawei.com \
    --to=marat.khalili@huawei.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=thomas@monjalon.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox