diff mbox series

[FFmpeg-devel] gdigrab: Fix hwnd parameter issues

Message ID 20231218085426.3955262-1-martin@martin.st
State Accepted
Commit f80b2ce9fa3235ce0e627d54df25e3f6cc929c05
Headers show
Series [FFmpeg-devel] gdigrab: Fix hwnd parameter issues | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Martin Storsjö Dec. 18, 2023, 8:54 a.m. UTC
Converting from an integer to HWND (which is a pointer) requires
an explicit cast, otherwise Clang errors out like this:

    src/libavdevice/gdigrab.c:280:14: error: incompatible integer to pointer conversion assigning to 'HWND' (aka 'struct HWND__ *') from 'long' [-Wint-conversion]
      280 |         hwnd = strtol(name, &p, 0);
          |              ^ ~~~~~~~~~~~~~~~~~~~

(With GCC and MSVC, this was a mere warning, but with recent Clang,
this is an error.)

After adding a cast, all compilers also warn something like this:

    src/libavdevice/gdigrab.c:280:16: warning: cast to 'HWND' (aka 'struct HWND__ *') from smaller integer type 'long' [-Wint-to-pointer-cast]
      280 |         hwnd = (HWND) strtol(name, &p, 0);
          |                ^~~~~~~~~~~~~~~~~~~~~~~~~~

On Windows, long types are 32 bit, so to get a usable pointer, we
need to use long long. And interpret it as unsigned long long
while at it - i.e. using strtoull.

Finally, right above it, the code triggered the following warning:

    src/libavdevice/gdigrab.c:278:15: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
      278 |         char *p;
          |               ^
---
 libavdevice/gdigrab.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

James Almer Dec. 18, 2023, 10:05 p.m. UTC | #1
On 12/18/2023 5:54 AM, Martin Storsjö wrote:
> Converting from an integer to HWND (which is a pointer) requires
> an explicit cast, otherwise Clang errors out like this:
> 
>      src/libavdevice/gdigrab.c:280:14: error: incompatible integer to pointer conversion assigning to 'HWND' (aka 'struct HWND__ *') from 'long' [-Wint-conversion]
>        280 |         hwnd = strtol(name, &p, 0);
>            |              ^ ~~~~~~~~~~~~~~~~~~~
> 
> (With GCC and MSVC, this was a mere warning, but with recent Clang,
> this is an error.)
> 
> After adding a cast, all compilers also warn something like this:
> 
>      src/libavdevice/gdigrab.c:280:16: warning: cast to 'HWND' (aka 'struct HWND__ *') from smaller integer type 'long' [-Wint-to-pointer-cast]
>        280 |         hwnd = (HWND) strtol(name, &p, 0);
>            |                ^~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> On Windows, long types are 32 bit, so to get a usable pointer, we
> need to use long long. And interpret it as unsigned long long
> while at it - i.e. using strtoull.
> 
> Finally, right above it, the code triggered the following warning:
> 
>      src/libavdevice/gdigrab.c:278:15: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
>        278 |         char *p;
>            |               ^
> ---
>   libavdevice/gdigrab.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavdevice/gdigrab.c b/libavdevice/gdigrab.c
> index 41ef370f2b..b2858ecd89 100644
> --- a/libavdevice/gdigrab.c
> +++ b/libavdevice/gdigrab.c
> @@ -274,10 +274,10 @@ gdigrab_read_header(AVFormatContext *s1)
>       } else if (!strcmp(filename, "desktop")) {
>           hwnd = NULL;
>       } else if (!strncmp(filename, "hwnd=", 5)) {
> -        name = filename + 5;
>           char *p;
> +        name = filename + 5;
>   
> -        hwnd = strtol(name, &p, 0);
> +        hwnd = (HWND) strtoull(name, &p, 0);
>   
>           if (p == NULL || p == name || p[0] == '\0')
>           {

Should be ok.
Stefano Sabatini Dec. 18, 2023, 10:08 p.m. UTC | #2
On date Monday 2023-12-18 19:05:17 -0300, James Almer wrote:
> On 12/18/2023 5:54 AM, Martin Storsjö wrote:
> > Converting from an integer to HWND (which is a pointer) requires
> > an explicit cast, otherwise Clang errors out like this:
> > 
> >      src/libavdevice/gdigrab.c:280:14: error: incompatible integer to pointer conversion assigning to 'HWND' (aka 'struct HWND__ *') from 'long' [-Wint-conversion]
> >        280 |         hwnd = strtol(name, &p, 0);
> >            |              ^ ~~~~~~~~~~~~~~~~~~~
> > 
> > (With GCC and MSVC, this was a mere warning, but with recent Clang,
> > this is an error.)
> > 
> > After adding a cast, all compilers also warn something like this:
> > 
> >      src/libavdevice/gdigrab.c:280:16: warning: cast to 'HWND' (aka 'struct HWND__ *') from smaller integer type 'long' [-Wint-to-pointer-cast]
> >        280 |         hwnd = (HWND) strtol(name, &p, 0);
> >            |                ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > On Windows, long types are 32 bit, so to get a usable pointer, we
> > need to use long long. And interpret it as unsigned long long
> > while at it - i.e. using strtoull.
> > 
> > Finally, right above it, the code triggered the following warning:
> > 
> >      src/libavdevice/gdigrab.c:278:15: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
> >        278 |         char *p;
> >            |               ^
> > ---
> >   libavdevice/gdigrab.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavdevice/gdigrab.c b/libavdevice/gdigrab.c
> > index 41ef370f2b..b2858ecd89 100644
> > --- a/libavdevice/gdigrab.c
> > +++ b/libavdevice/gdigrab.c
> > @@ -274,10 +274,10 @@ gdigrab_read_header(AVFormatContext *s1)
> >       } else if (!strcmp(filename, "desktop")) {
> >           hwnd = NULL;
> >       } else if (!strncmp(filename, "hwnd=", 5)) {
> > -        name = filename + 5;
> >           char *p;
> > +        name = filename + 5;
> > -        hwnd = strtol(name, &p, 0);
> > +        hwnd = (HWND) strtoull(name, &p, 0);
> >           if (p == NULL || p == name || p[0] == '\0')
> >           {
> 
> Should be ok.

Also LGTM, thanks.
Martin Storsjö Dec. 18, 2023, 10:18 p.m. UTC | #3
On Mon, 18 Dec 2023, Stefano Sabatini wrote:

> On date Monday 2023-12-18 19:05:17 -0300, James Almer wrote:
>> On 12/18/2023 5:54 AM, Martin Storsjö wrote:
>>> Converting from an integer to HWND (which is a pointer) requires
>>> an explicit cast, otherwise Clang errors out like this:
>>>
>>>      src/libavdevice/gdigrab.c:280:14: error: incompatible integer to pointer conversion assigning to 'HWND' (aka 'struct HWND__ *') from 'long' [-Wint-conversion]
>>>        280 |         hwnd = strtol(name, &p, 0);
>>>            |              ^ ~~~~~~~~~~~~~~~~~~~
>>>
>>> (With GCC and MSVC, this was a mere warning, but with recent Clang,
>>> this is an error.)
>>>
>>> After adding a cast, all compilers also warn something like this:
>>>
>>>      src/libavdevice/gdigrab.c:280:16: warning: cast to 'HWND' (aka 'struct HWND__ *') from smaller integer type 'long' [-Wint-to-pointer-cast]
>>>        280 |         hwnd = (HWND) strtol(name, &p, 0);
>>>            |                ^~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> On Windows, long types are 32 bit, so to get a usable pointer, we
>>> need to use long long. And interpret it as unsigned long long
>>> while at it - i.e. using strtoull.
>>>
>>> Finally, right above it, the code triggered the following warning:
>>>
>>>      src/libavdevice/gdigrab.c:278:15: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
>>>        278 |         char *p;
>>>            |               ^
>>> ---
>>>   libavdevice/gdigrab.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavdevice/gdigrab.c b/libavdevice/gdigrab.c
>>> index 41ef370f2b..b2858ecd89 100644
>>> --- a/libavdevice/gdigrab.c
>>> +++ b/libavdevice/gdigrab.c
>>> @@ -274,10 +274,10 @@ gdigrab_read_header(AVFormatContext *s1)
>>>       } else if (!strcmp(filename, "desktop")) {
>>>           hwnd = NULL;
>>>       } else if (!strncmp(filename, "hwnd=", 5)) {
>>> -        name = filename + 5;
>>>           char *p;
>>> +        name = filename + 5;
>>> -        hwnd = strtol(name, &p, 0);
>>> +        hwnd = (HWND) strtoull(name, &p, 0);
>>>           if (p == NULL || p == name || p[0] == '\0')
>>>           {
>>
>> Should be ok.
>
> Also LGTM, thanks.

Pushed now, thanks!

// Martin
diff mbox series

Patch

diff --git a/libavdevice/gdigrab.c b/libavdevice/gdigrab.c
index 41ef370f2b..b2858ecd89 100644
--- a/libavdevice/gdigrab.c
+++ b/libavdevice/gdigrab.c
@@ -274,10 +274,10 @@  gdigrab_read_header(AVFormatContext *s1)
     } else if (!strcmp(filename, "desktop")) {
         hwnd = NULL;
     } else if (!strncmp(filename, "hwnd=", 5)) {
-        name = filename + 5;
         char *p;
+        name = filename + 5;
 
-        hwnd = strtol(name, &p, 0);
+        hwnd = (HWND) strtoull(name, &p, 0);
 
         if (p == NULL || p == name || p[0] == '\0')
         {