From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Rosen Penev <rosenp@gmail.com>
Cc: linux-media@vger.kernel.org
Subject: Re: [PATCH 7/8] [clang-tidy] fix mismatching declarations
Date: Mon, 27 Jul 2020 08:58:23 +0200 [thread overview]
Message-ID: <20200727085823.78fa67c8@coco.lan> (raw)
In-Reply-To: <20200727031456.232955-8-rosenp@gmail.com>
Em Sun, 26 Jul 2020 20:14:55 -0700
Rosen Penev <rosenp@gmail.com> escreveu:
> Found with readability-inconsistent-declaration-parameter-name
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> lib/include/libdvbv5/atsc_eit.h | 4 +-
> lib/include/libdvbv5/cat.h | 4 +-
> lib/include/libdvbv5/descriptors.h | 2 +-
> lib/include/libdvbv5/dvb-demux.h | 2 +-
> lib/include/libdvbv5/dvb-dev.h | 2 +-
> lib/include/libdvbv5/dvb-fe.h | 2 +-
> lib/include/libdvbv5/dvb-file.h | 4 +-
> lib/include/libdvbv5/dvb-scan.h | 16 +++----
> lib/include/libdvbv5/eit.h | 4 +-
> lib/include/libdvbv5/header.h | 4 +-
> lib/include/libdvbv5/mgt.h | 4 +-
> lib/include/libdvbv5/mpeg_pes.h | 2 +-
> lib/include/libdvbv5/nit.h | 6 +--
> lib/include/libdvbv5/pat.h | 4 +-
> lib/include/libdvbv5/pmt.h | 4 +-
> lib/include/libdvbv5/sdt.h | 4 +-
> lib/include/libdvbv5/vct.h | 4 +-
> lib/include/libv4l2.h | 2 +-
> lib/libdvbv5/parse_string.h | 2 +-
> lib/libv4lconvert/libv4lconvert-priv.h | 48 +++++++++----------
> .../processing/libv4lprocessing.h | 2 +-
> lib/libv4lconvert/tinyjpeg.h | 2 +-
> utils/common/v4l-stream.h | 4 +-
> utils/keytable/bpf.h | 6 +--
> utils/libcecutil/cec-log.cpp | 12 ++---
> 25 files changed, 75 insertions(+), 75 deletions(-)
>
> diff --git a/lib/include/libdvbv5/atsc_eit.h b/lib/include/libdvbv5/atsc_eit.h
> index 5e52087c..18ae599d 100644
> --- a/lib/include/libdvbv5/atsc_eit.h
> +++ b/lib/include/libdvbv5/atsc_eit.h
> @@ -192,7 +192,7 @@ ssize_t atsc_table_eit_init(struct dvb_v5_fe_parms *parms, const uint8_t *buf,
> *
> * @param table pointer to struct atsc_table_eit to be freed
> */
> -void atsc_table_eit_free(struct atsc_table_eit *table);
> +void atsc_table_eit_free(struct atsc_table_eit *eit);
>
> /**
> * @brief Prints the content of the ATSC EIT table
> @@ -202,7 +202,7 @@ void atsc_table_eit_free(struct atsc_table_eit *table);
> * @param table pointer to struct atsc_table_eit
> */
> void atsc_table_eit_print(struct dvb_v5_fe_parms *parms,
> - struct atsc_table_eit *table);
> + struct atsc_table_eit *eit);
A change like that will break the documentation build, as it relies
on "table" name for this parameter:
/**
* @brief Prints the content of the ATSC EIT table
* @ingroup dvb_table
*
* @param parms struct dvb_v5_fe_parms pointer to the opened device
* @param table pointer to struct atsc_table_eit
*/
void atsc_table_eit_print(struct dvb_v5_fe_parms *parms,
struct atsc_table_eit *table);
So, if this is willing to be changed, the kerneldoc header would
need a similar change...
> diff --git a/lib/include/libdvbv5/dvb-fe.h b/lib/include/libdvbv5/dvb-fe.h
> index 96657013..4bd94108 100644
> --- a/lib/include/libdvbv5/dvb-fe.h
> +++ b/lib/include/libdvbv5/dvb-fe.h
> @@ -732,7 +732,7 @@ int dvb_fe_is_satellite(uint32_t delivery_system);
> * "COUNTRY" property in dvb_fe_set_parm() overrides the setting.
> */
> int dvb_fe_set_default_country(struct dvb_v5_fe_parms *parms,
> - const char *country);
> + const char *cc);
>
> #ifdef __cplusplus
> }
...yet, some of those changes are not ok.
I mean, while it is OK to use "cc" inside the function implementation
(it is an alias for Country code), at the headers - and at the
documentation, which is created by Doxygen, it should keep a better
description.
Btw, the main reason why the headers don't match the implementation
is because those parameter names changed when we added support for
Doxygen. The goal was to have parameter names that would be
clearer about what the parameter was meant for.
So, if you want to have both using the same name, specially for a
parameter like "country", the change should be done inside the
implementation, and not at the header.
Just to mention, I'm OK on keeping both declaration and usage in
sync, although this change shouldn't affect C++11, as the
implementation is in C.
Thanks,
Mauro
next prev parent reply other threads:[~2020-07-27 6:58 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-27 3:14 [PATCH 0/8] v4l-utils: C++11 modernization Rosen Penev
2020-07-27 3:14 ` [PATCH 1/8] fix GCC enum warning Rosen Penev
2020-08-03 10:13 ` Hans Verkuil
2020-07-27 3:14 ` [PATCH 2/8] [clang-tidy] convert to range based loops Rosen Penev
2020-07-27 3:14 ` [PATCH 3/8] [clang-tidy] use auto Rosen Penev
2020-07-27 3:14 ` [PATCH 4/8] [clang-tidy] use using instead of typedef Rosen Penev
2020-07-27 3:14 ` [PATCH 5/8] [clang-tidy] use emplace_back Rosen Penev
2020-07-27 3:14 ` [PATCH 6/8] [clang-tidy] convert files to reference Rosen Penev
2020-07-27 3:14 ` [PATCH 7/8] [clang-tidy] fix mismatching declarations Rosen Penev
2020-07-27 6:58 ` Mauro Carvalho Chehab [this message]
2020-07-27 3:14 ` [PATCH 8/8] [clang-tidy] use explicit for single argument constructors Rosen Penev
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=20200727085823.78fa67c8@coco.lan \
--to=mchehab+huawei@kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=rosenp@gmail.com \
/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;
as well as URLs for NNTP newsgroup(s).