ddxLoad is unreadable and unsafe. With the windows integration lot of security issues must be adressed and the code should be cleaned up to make it more readable and maintainable - #ifdef __UNIXOS2__ and WIN32 blocks should be reduced to a minimum - more buffers should be checked for size before copying data to them - the xkbcomp commandline should be allocated dynamicly (using Xprintf instead of sprintf on static buffer)
Created attachment 1645 [details] [review] cleanup of ddxLoad.c * xc/programs/Xserver/xkb/ddxLoad.c: cleanup some #ifdef __UNIXOS2__ and WIN32 blocks. make OutputDirectory check the size of the buffer quote all file and pathnames in the xkbcomp commandline use PATH_MAX*4 for commandline buffer
Comment on attachment 1645 [details] [review] cleanup of ddxLoad.c comitted to HEAD /cvs/xorg/xc/ChangeLog,v <-- ChangeLog new revision: 1.649; previous revision: 1.648 /cvs/xorg/xc/programs/Xserver/xkb/ddxLoad.c,v <-- ddxLoad.c new revision: 1.8; previous revision: 1.7
Created attachment 1647 [details] [review] cleanup ddxList.c * xc/programs/Xserver/xkb/ddxList.c: * xc/programs/Xserver/xkb/ddxLoad.c: export Win32System and Win32TempDir remove #ifdef WIN32 block for building xkbcomp commandline create win32 tempfile in system tempdir use PATH_MAX*4 for commandline buffer unlink tempfile again
Comment on attachment 1647 [details] [review] cleanup ddxList.c commited to HEAD /cvs/xorg/xc/programs/Xserver/xkb/ddxList.c,v <-- ddxList.c new revision: 1.4; previous revision: 1.3 /cvs/xorg/xc/programs/Xserver/xkb/ddxLoad.c,v <-- ddxLoad.c new revision: 1.9; previous revision: 1.8
Created attachment 1648 [details] [review] Allocate xkbcomp cmdline buffer dynamicly The buffer for the xkbcomp commandline is allocated dynamicly via Xprintf. This removes the limits of the static buffer and the extra calculation of required space which can easily get out of sync with the actual sprintf usage Any objections?
Created attachment 1649 [details] [review] Allocate xkbcomp cmdline buffer dynamicly removed another buffer length calculation which I missed in the previous patch
xprintf.c lack a license. Please add an explicit one.
added license statement to xprintf.c /cvs/xorg/xc/ChangeLog,v <-- ChangeLog new revision: 1.681; previous revision: 1.680 /cvs/xorg/xc/programs/Xserver/os/xprintf.c,v <-- xprintf.c new revision: 1.2; previous revision: 1.1
Comment on attachment 1649 [details] [review] Allocate xkbcomp cmdline buffer dynamicly /cvs/xorg/xc/ChangeLog,v <-- ChangeLog new revision: 1.737; previous revision: 1.736 /cvs/xorg/xc/programs/Xserver/xkb/ddxLoad.c,v <-- programs/Xserver/xkb/ddxLoad.c new revision: 1.10; previous revision: 1.9 /cvs/xorg/xc/programs/Xserver/xkb/ddxList.c,v <-- programs/Xserver/xkb/ddxList.c new revision: 1.5; previous revision: 1.4
commited to HEAD. Marking as fixed...
Created attachment 1817 [details] [review] Link libos.a after libxkb.a The use of Xprintf in libxkb.a requires libos.a to be linked after libxkb.a on some systems. The patch adds a makro dependLib(x) which currently evaluates to x but can also be configured to be silent for some platforms and adds dependLib($(OS)) to XKBEXT to make sure libos.a is added after libxkb.a too. Maybe the whole linking needs to be reworked. we currently have circular dependencies between the libraries. dix need symbols from ddx, ddx needs symbols from os, os needs symbols from dix and so forth.
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.