Bug 14735 - xorg/driver/xf86-input-magictouch - Dont call nonexistent function and compile warning fixes
Summary: xorg/driver/xf86-input-magictouch - Dont call nonexistent function and compil...
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Input/magictouch (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords: janitor, patch
Depends on:
Blocks:
 
Reported: 2008-02-28 21:49 UTC by Paulo César Pereira de Andrade
Modified: 2008-05-04 19:39 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
0001-Remove-macro-defining-strdup-to-xf86strdup.patch (916 bytes, patch)
2008-02-28 21:49 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review
0002-Fix-typo-emoveEnabledDevice-RemoveEnabledDevice.patch (809 bytes, patch)
2008-02-28 21:50 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review
0001-Don-t-call-non-existing-function-and-compile-warning.patch (2.68 KB, patch)
2008-03-15 22:49 UTC, Paulo César Pereira de Andrade
no flags Details | Splinter Review

Description Paulo César Pereira de Andrade 2008-02-28 21:49:55 UTC
Created attachment 14694 [details] [review]
0001-Remove-macro-defining-strdup-to-xf86strdup.patch

I posted this patch at xorg@ some time ago. Posting again
to avoid it being lost.

   Also a bit unsure about what would be the proper way to
check for xf86_ansic functions, as adding a macro for checking
the proper sdk header, and cut&pasting everywhere may not
be the better way, so it checks XORG_VERSION_CURRENT, but
that value has not yet been updated in git master...
Comment 1 Paulo César Pereira de Andrade 2008-02-28 21:50:15 UTC
Created attachment 14695 [details] [review]
0002-Fix-typo-emoveEnabledDevice-RemoveEnabledDevice.patch
Comment 2 Peter Hutterer 2008-02-28 22:55:43 UTC
(In reply to comment #1)
> Created an attachment (id=14695) [details]
> 0002-Fix-typo-emoveEnabledDevice-RemoveEnabledDevice.patch
> 

thanks. pushed as bbd274235f80cc8858f27d232f4218de2792250e. 


(In reply to comment #0)
>    Also a bit unsure about what would be the proper way to
> check for xf86_ansic functions, as adding a macro for checking
> the proper sdk header, and cut&pasting everywhere may not
> be the better way, so it checks XORG_VERSION_CURRENT, but
> that value has not yet been updated in git master...

I won't push this patch. Change all notions of xf86strdup to strdup seems to make more sense than this ifdef game. we don't compile on anything not having libc anyway.
Comment 3 Paulo César Pereira de Andrade 2008-02-28 23:06:54 UTC
> (In reply to comment #0)
> >    Also a bit unsure about what would be the proper way to
> > check for xf86_ansic functions, as adding a macro for checking
> > the proper sdk header, and cut&pasting everywhere may not
> > be the better way, so it checks XORG_VERSION_CURRENT, but
> > that value has not yet been updated in git master...
> 
> I won't push this patch. Change all notions of xf86strdup to strdup seems to
> make more sense than this ifdef game. we don't compile on anything not having
> libc anyway.

  Probably it makes more sense. There isn't much point in supporting
xf86_ansic now as there is only one loader, and the only reason one
could want to wrap libc is some sort of fake/weak security to avoid
binary only drivers trying to do more than they should (most likely
with super user priviledges)...
Comment 4 Paulo César Pereira de Andrade 2008-03-15 22:48:14 UTC
patch 2 was already applied in commit bbd274235f80cc8858f27d232f4218de2792250e
Comment 5 Paulo César Pereira de Andrade 2008-03-15 22:49:46 UTC
Created attachment 15172 [details] [review]
0001-Don-t-call-non-existing-function-and-compile-warning.patch

  This patch follows the model in video drivers, i.e. no reason
to have tests to check for the presence of xf86ansic, just don't
call xf86<libc> functions directly.
Comment 6 Peter Hutterer 2008-05-01 17:46:28 UTC
(In reply to comment #5)
> Created an attachment (id=15172) [details]
> 0001-Don-t-call-non-existing-function-and-compile-warning.patch
> 
>   This patch follows the model in video drivers, i.e. no reason
> to have tests to check for the presence of xf86ansic, just don't
> call xf86<libc> functions directly.

Paulo:

+#if 0
 	int            status_line;
+#endif

why?


+	ErrorF("xf86MagicReadInput: Touch Controller non inizializzato\n")

I think english would be more appropriate (I know it was spanish (?) before, but still).
Comment 7 Paulo César Pereira de Andrade 2008-05-01 19:09:56 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > Created an attachment (id=15172) [details] [details]
> > 0001-Don-t-call-non-existing-function-and-compile-warning.patch
> > 
> >   This patch follows the model in video drivers, i.e. no reason
> > to have tests to check for the presence of xf86ansic, just don't
> > call xf86<libc> functions directly.
> 
> Paulo:
> 
> +#if 0
>         int            status_line;
> +#endif
> 
> why?

  The code that uses it is #if 0'ed, so I followed pattern. Otherwise
gcc generates a (harmless) warning about an unused variabled.

> +       ErrorF("xf86MagicReadInput: Touch Controller non inizializzato\n")
> 
> I think english would be more appropriate (I know it was spanish (?) before,
> but still).

  It is italian, but my change was just a guess, as the call had the
format "<<%s[%d]>>", but no arguments. The other calls with
that format receive the expected __FILE__, __LINE__ arguments, so
maybe it would be a more proper patch to match driver author idea.

  The text should say: "Touch Controller not initialized" (but just
guessing like I would guess any other Latin derived language :-)

  I think the only other information I can find about it is the logs in
http://cvsweb.xfree86.org/cvsweb/xc/programs/Xserver/hw/xfree86/input/magictouch/xf86MagicTouch.c

Damnit, sorry I did not remember it was me who added it :-)

http://cvsweb.xfree86.org/cvsweb/xc/programs/Xserver/hw/xfree86/CHANGELOG?rev=HEAD
doesn't have any more useful information either...

  I think it was posted to the patches list, and was one of the several
patches I applied, but never ever talked with the original author...

http://marc.info/?l=xfree86-devel&w=2 could be a good source to try to
find information about other old/unmantained input drivers...
Comment 8 Peter Hutterer 2008-05-04 19:39:10 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Created an attachment (id=15172) [details] [details] [details]
> > > 0001-Don-t-call-non-existing-function-and-compile-warning.patch
> > > 
> > >   This patch follows the model in video drivers, i.e. no reason
> > > to have tests to check for the presence of xf86ansic, just don't
> > > call xf86<libc> functions directly.
> > 
> > Paulo:
> > 
> > +#if 0
> >         int            status_line;
> > +#endif
> > 
> > why?
> 
>   The code that uses it is #if 0'ed, so I followed pattern. Otherwise
> gcc generates a (harmless) warning about an unused variabled.

fair enough.

> 
> > +       ErrorF("xf86MagicReadInput: Touch Controller non inizializzato\n")
> > 
> > I think english would be more appropriate (I know it was spanish (?) before,
> > but still).
> 
>   It is italian, but my change was just a guess, as the call had the
> format "<<%s[%d]>>", but no arguments. The other calls with
> that format receive the expected __FILE__, __LINE__ arguments, so
> maybe it would be a more proper patch to match driver author idea.
> 
>   The text should say: "Touch Controller not initialized" (but just
> guessing like I would guess any other Latin derived language :-)

Yeah, I thought about fixing it, but then again that would require to translate everything and I don't have the time for that now.


pushed as c616070ccdafd1dede1eb263bb480c7cf6cf6145.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.