result parameter for ScreenWakeupHandlerProcPtr should be unsigned long

Submitted by Kevin Brace on Dec. 13, 2018, 8:47 p.m.

Details

Message ID 1544734022-17192-1-git-send-email-kevinbrace@gmx.com
State New
Series "result parameter for ScreenWakeupHandlerProcPtr should be unsigned long"
Headers show

Commit Message

Kevin Brace Dec. 13, 2018, 8:47 p.m.
The result parameter defined inside scrnintstr.h for
ScreenWakeupHandlerProcPtr should be unsigned long type not int type.
This led to wrong pointer type compilation warnings when compiling S3
Savage DDX Version 2.3.9 for ABI_VIDEODRV_VERSION >= 23.

Signed-off-by: Kevin Brace <kevinbrace@gmx.com>
---
 include/scrnintstr.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/include/scrnintstr.h b/include/scrnintstr.h
index 9b663fa..111cab5 100644
--- a/include/scrnintstr.h
+++ b/include/scrnintstr.h
@@ -278,7 +278,7 @@  typedef void (*ScreenBlockHandlerProcPtr) (ScreenPtr pScreen,
  * > 0 - activity
  */
 typedef void (*ScreenWakeupHandlerProcPtr) (ScreenPtr pScreen,
-                                            int result);
+                                            unsigned long result);
 
 typedef Bool (*CreateScreenResourcesProcPtr) (ScreenPtr /*pScreen */ );
 

Comments

Dave Airlie Jan. 3, 2019, 9:14 p.m.
On Fri, 14 Dec 2018 at 06:47, Kevin Brace <kevinbrace@gmx.com> wrote:
>
> The result parameter defined inside scrnintstr.h for
> ScreenWakeupHandlerProcPtr should be unsigned long type not int type.
> This led to wrong pointer type compilation warnings when compiling S3
> Savage DDX Version 2.3.9 for ABI_VIDEODRV_VERSION >= 23.
>
> Signed-off-by: Kevin Brace <kevinbrace@gmx.com>

I think Alan already mentioned this, but it should just be fixed in the driver.

Dave.

> ---
>  include/scrnintstr.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/scrnintstr.h b/include/scrnintstr.h
> index 9b663fa..111cab5 100644
> --- a/include/scrnintstr.h
> +++ b/include/scrnintstr.h
> @@ -278,7 +278,7 @@ typedef void (*ScreenBlockHandlerProcPtr) (ScreenPtr pScreen,
>   * > 0 - activity
>   */
>  typedef void (*ScreenWakeupHandlerProcPtr) (ScreenPtr pScreen,
> -                                            int result);
> +                                            unsigned long result);
>
>  typedef Bool (*CreateScreenResourcesProcPtr) (ScreenPtr /*pScreen */ );
>
> --
> 2.7.4
>
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
Kevin Brace Jan. 9, 2019, 5:56 p.m.
Hi Dave,

I do see what is going on, but was there a mistake in the X Server header file when the ABI was changed?
Who else uses ScreenWakeupHandlerProcPtr and does another device driver have this issue?

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Thursday, January 03, 2019 at 3:14 PM
> From: "Dave Airlie" <airlied@gmail.com>
> To: "Kevin Brace" <kevinbrace@gmx.com>
> Cc: xorg-devel <xorg-devel@lists.x.org>
> Subject: Re: [PATCH] result parameter for ScreenWakeupHandlerProcPtr should be unsigned long
>
> On Fri, 14 Dec 2018 at 06:47, Kevin Brace <kevinbrace@gmx.com> wrote:
> >
> > The result parameter defined inside scrnintstr.h for
> > ScreenWakeupHandlerProcPtr should be unsigned long type not int type.
> > This led to wrong pointer type compilation warnings when compiling S3
> > Savage DDX Version 2.3.9 for ABI_VIDEODRV_VERSION >= 23.
> >
> > Signed-off-by: Kevin Brace <kevinbrace@gmx.com>
> 
> I think Alan already mentioned this, but it should just be fixed in the driver.
> 
> Dave.
> 
> > ---
> >  include/scrnintstr.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/scrnintstr.h b/include/scrnintstr.h
> > index 9b663fa..111cab5 100644
> > --- a/include/scrnintstr.h
> > +++ b/include/scrnintstr.h
> > @@ -278,7 +278,7 @@ typedef void (*ScreenBlockHandlerProcPtr) (ScreenPtr pScreen,
> >   * > 0 - activity
> >   */
> >  typedef void (*ScreenWakeupHandlerProcPtr) (ScreenPtr pScreen,
> > -                                            int result);
> > +                                            unsigned long result);
> >
> >  typedef Bool (*CreateScreenResourcesProcPtr) (ScreenPtr /*pScreen */ );
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > xorg-devel@lists.x.org: X.Org development
> > Archives: http://lists.x.org/archives/xorg-devel
> > Info: https://lists.x.org/mailman/listinfo/xorg-devel
>
Matt Turner Jan. 9, 2019, 7:02 p.m.
On Wed, Jan 9, 2019 at 10:03 AM Kevin Brace <kevinbrace@gmx.com> wrote:
>
> Hi Dave,
>
> I do see what is going on, but was there a mistake in the X Server header file when the ABI was changed?
> Who else uses ScreenWakeupHandlerProcPtr and does another device driver have this issue?

Can we please try to keep discussion of one issue to one thread? This
is now the third separate thread on this list about this issue.

Also, please do not top quote.

As I said in the other thread, the result parameter is not even used
by -savage, so just change -savage and move on. I don't remember
finding any other drivers that used ScreenWakeupHandlerProcPtr, but
you can certainly confirm by cloning the repos and grepping. There's
no need to ask a question if you can easily answer it yourself
definitively.

I don't know that the s/unsigned long/int/ change (which was done in
xserver commit fb0802113b4c57819cba15d64baf79bf4148607e) was a mistake
-- seems more like a simplification while the ABI was being changes
anyway.