diff mbox

[FFmpeg-devel,2.0/3] libavformat/protocols.c: avio_enum_protocols(): Add more const-correctness

Message ID 21804bb93c4544208a445d1286efa8b6@gmail.com
State New
Headers show

Commit Message

Michael Witten Aug. 11, 2021, 7 p.m. UTC
This commit adds 'const' qualifiers to the parameters.

---
 libavformat/avio.h      | 2 +-
 libavformat/protocols.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Andreas Rheinhardt Aug. 11, 2021, 9:07 p.m. UTC | #1
Michael Witten:
> This commit adds 'const' qualifiers to the parameters.
> 
> ---
>  libavformat/avio.h      | 2 +-
>  libavformat/protocols.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/avio.h b/libavformat/avio.h
> index 0b35409787..3b92cf742a 100644
> --- a/libavformat/avio.h
> +++ b/libavformat/avio.h
> @@ -786,7 +786,7 @@ int avio_close_dyn_buf(AVIOContext *s, uint8_t **pbuffer);
>   *
>   * @return A static string containing the name of current protocol or NULL
>   */
> -const char *avio_enum_protocols(void **opaque, int output);
> +const char *avio_enum_protocols(void **const opaque, const int output);
>  
>  /**
>   * Get AVClass by names of available protocols.
> diff --git a/libavformat/protocols.c b/libavformat/protocols.c
> index e0b3405ab8..e671c5ab6a 100644
> --- a/libavformat/protocols.c
> +++ b/libavformat/protocols.c
> @@ -91,7 +91,7 @@ const AVClass *ff_urlcontext_child_class_iterate(void **iter)
>      return ret;
>  }
>  
> -const char *avio_enum_protocols(void **opaque, int output)
> +const char *avio_enum_protocols(void **const opaque, const int output)
>  {
>      const URLProtocol *const *p = *opaque;
>  
> 
This thing makes nothing more const-correct at all; C uses call be
value, so we only deal with our own copy of opaque which we may modify
as we please.
The reason for the const-incorrectness lies in the fact that this
function makes *opaque point to something const (namely an entry in a
const list of pointers (each pointing to a const protocol, but that is
irrelevant)), so that the user now has a pointer whose pointed to-type
is not const-qualified, despite the actual target being const.
See here:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20190821090438.10260-2-andreas.rheinhardt@gmail.com/
for how to fix the const-correctness if one wants to keep the general
design of the functions. See libavformat/allformats.c,
libavcodec/allcodecs.c, libavfilter/allfilters.c for the other approach
(that involves storing an index in the pointer).

- Andreas
Michael Witten Aug. 11, 2021, 9:37 p.m. UTC | #2
| Michael Witten:
|
|   > -const char *avio_enum_protocols(void **opaque, int output)
|   > +const char *avio_enum_protocols(void **const opaque, const int output)
|
| Andreas Rheinhardt:
|
|   > This thing makes nothing more const-correct at all; C uses call be
|   > value, so we only deal with our own copy of opaque which we may modify
|   > as we please.
|   > The reason for the const-incorrectness lies in the fact that this
|   > function makes *opaque point to something const (namely an entry in a
|   > const list of pointers (each pointing to a const protocol, but that is
|   > irrelevant)), so that the user now has a pointer whose pointed to-type
|   > is not const-qualified, despite the actual target being const.
|   > See here:
|   > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20190821090438.10260-2-andreas.rheinhardt@gmail.com/
|   > for how to fix the const-correctness if one wants to keep the general
|   > design of the functions. See libavformat/allformats.c,
|   > libavcodec/allcodecs.c, libavfilter/allfilters.c for the other approach
|   > (that involves storing an index in the pointer).
|   >
|   > - Andreas

* The general design of the functions has not been altered by this series.

* The previous patch fixed the const incorrectness as warned by the compiler.
  This patch merely improves the const correctness in general.

* We don't want to modify those parameters as we please, and so we'd like
  to ask the compiler to remind us of that fact as necessary.

  It's const-correct in the sense that it expresses the following fact: Those
  variables are not going to be changed; if there is no intention to modify a
  variable, then it's good practice to mark that variable as const.

  Because this function is being edited, it's a good time to sprinkle
  the 'const' keyword around, as a matter of mental hygiene.

Sincerely,
Michael Witten
diff mbox

Patch

diff --git a/libavformat/avio.h b/libavformat/avio.h
index 0b35409787..3b92cf742a 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -786,7 +786,7 @@  int avio_close_dyn_buf(AVIOContext *s, uint8_t **pbuffer);
  *
  * @return A static string containing the name of current protocol or NULL
  */
-const char *avio_enum_protocols(void **opaque, int output);
+const char *avio_enum_protocols(void **const opaque, const int output);
 
 /**
  * Get AVClass by names of available protocols.
diff --git a/libavformat/protocols.c b/libavformat/protocols.c
index e0b3405ab8..e671c5ab6a 100644
--- a/libavformat/protocols.c
+++ b/libavformat/protocols.c
@@ -91,7 +91,7 @@  const AVClass *ff_urlcontext_child_class_iterate(void **iter)
     return ret;
 }
 
-const char *avio_enum_protocols(void **opaque, int output)
+const char *avio_enum_protocols(void **const opaque, const int output)
 {
     const URLProtocol *const *p = *opaque;