From 922806a5aa6eafc432d6787495b475aaa3f1790d Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 00:14:51 -0800 Subject: [PATCH 01/16] Xserver-spec: Update Memory Management functions Xalloc, Xrealloc, & Xfree are deprecated now ALLOCATE_LOCAL is removed due to stack overflow issues Signed-off-by: Alan Coopersmith Reviewed-by: Julien Cristau Reviewed-by: Jeremy Huddleston --- doc/xml/Xserver-spec.xml | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/doc/xml/Xserver-spec.xml b/doc/xml/Xserver-spec.xml index ae15346f7..dbf088304 100644 --- a/doc/xml/Xserver-spec.xml +++ b/doc/xml/Xserver-spec.xml @@ -1215,20 +1215,12 @@ library is contained in dix/dixfonts.c Memory Management Memory management is based on functions in the C runtime library. -Xalloc(), Xrealloc(), and Xfree() work just like malloc(), realloc(), -and free(), except that you can pass a null pointer to Xrealloc() to -have it allocate anew or pass a null pointer to Xfree() and nothing -will happen. The versions in the sample server also do some checking -that is useful for debugging. Consult a C runtime library reference +Xalloc(), Xrealloc(), and Xfree() are deprecated aliases for malloc(), +realloc(), and free(), and you should simply call the C library functions +directly. Consult a C runtime library reference manual for more details. -The macros ALLOCATE_LOCAL and DEALLOCATE_LOCAL are provided in -Xserver/include/os.h. These are useful if your compiler supports -alloca() (or some method of allocating memory from the stack); and are -defined appropriately on systems which support it. - - Treat memory allocation carefully in your implementation. Memory leaks can be very hard to find and are frustrating to a user. An X server could be running for days or weeks without being reset, just From f6c880b257a21a574cf1a47095cb39f32252802e Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 00:27:46 -0800 Subject: [PATCH 02/16] Xserver-spec: Replace deprecated resource id lookup functions Signed-off-by: Alan Coopersmith Reviewed-by: Julien Cristau Reviewed-by: Jeremy Huddleston --- doc/xml/Xserver-spec.xml | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/doc/xml/Xserver-spec.xml b/doc/xml/Xserver-spec.xml index dbf088304..1afae5dd1 100644 --- a/doc/xml/Xserver-spec.xml +++ b/doc/xml/Xserver-spec.xml @@ -442,18 +442,29 @@ and type; if skipFree is true, then the deleteFunc is not called. To look up a resource, use one of the following.
- pointer LookupIDByType(id, rtype) - XID id; - RESTYPE rtype; + int dixLookupResourceByType( + pointer *result, + XID id, + RESTYPE rtype, + ClientPtr client, + Mask access_mode); - pointer LookupIDByClass(id, classes) - XID id; - RESTYPE classes; + int dixLookupResourceByClass( + pointer *result, + XID id, + RESTYPE rclass, + ClientPtr client, + Mask access_mode);
-LookupIDByType finds a resource with the given id and exact type. -LookupIDByClass finds a resource with the given id whose type is -included in any one of the specified classes. +dixLookupResourceByType finds a resource with the given id and exact type. +dixLookupResourceByClass finds a resource with the given id whose type is +included in any one of the specified classes. +The client and access_mode must be provided to allow security extensions to +check if the client has the right privileges for the requested access. +The bitmask values defined in the dixaccess.h header are or'ed together +to define the requested access_mode. +
From de518c8f378ea31345c946693d58a26a493af603 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 00:30:37 -0800 Subject: [PATCH 03/16] Xserver-spec: Remove CreateCallbackList The function is defined as a static, so can't be called by anyone but AddCallback. Signed-off-by: Alan Coopersmith Reviewed-by: Julien Cristau Reviewed-by: Jeremy Huddleston --- doc/xml/Xserver-spec.xml | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/doc/xml/Xserver-spec.xml b/doc/xml/Xserver-spec.xml index 1afae5dd1..6dc291190 100644 --- a/doc/xml/Xserver-spec.xml +++ b/doc/xml/Xserver-spec.xml @@ -508,24 +508,6 @@ When CallCallbacks is invoked on the list, func will be called thusly: Now for the details. -
- - Bool CreateCallbackList(pcbl, cbfuncs) - CallbackListPtr *pcbl; - CallbackFuncsPtr cbfuncs; - -
-CreateCallbackList creates a callback list. We envision that this -function will be rarely used because the callback list is created -automatically (if it doesn't already exist) when the first call to -AddCallback is made on the list. The only reason to explicitly create -the callback list with this function is if you want to override the -implementation of some of the other operations on the list by passing -your own cbfuncs. You also lose something by explicit creation: you -introduce an order dependency during server startup because the list -must be created before any modules subscribe to it. Returns TRUE if -successful. -
Bool AddCallback(pcbl, callback, subscriber_data) From a410bf53798bdca43f99476a01ef27cabdf73e01 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 00:35:28 -0800 Subject: [PATCH 04/16] Xserver-spec: Update lists of macros LOOKUP_DRAWABLE & VERIFY_GC are no longer in dix.h, but WriteReplyToClient & WriteSwappedDataToClient are. Signed-off-by: Alan Coopersmith Reviewed-by: Julien Cristau Reviewed-by: Jeremy Huddleston --- doc/xml/Xserver-spec.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/xml/Xserver-spec.xml b/doc/xml/Xserver-spec.xml index 6dc291190..f35dd0c63 100644 --- a/doc/xml/Xserver-spec.xml +++ b/doc/xml/Xserver-spec.xml @@ -588,8 +588,9 @@ used here which takes the minor opcode from the normal place in the request There are a number of macros in Xserver/include/dix.h which are useful to the extension writer. Ones of particular interest are: REQUEST, REQUEST_SIZE_MATCH, REQUEST_AT_LEAST_SIZE, -REQUEST_FIXED_SIZE, LEGAL_NEW_RESOURCE, LOOKUP_DRAWABLE, VERIFY_GC, and +REQUEST_FIXED_SIZE, LEGAL_NEW_RESOURCE, and VALIDATE_DRAWABLE_AND_GC. Useful byte swapping macros can be found +in Xserver/include/dix.h: WriteReplyToClient and WriteSwappedDataToClient; and in Xserver/include/misc.h: lswapl, lswaps, LengthRestB, LengthRestS, LengthRestL, SwapRestS, SwapRestL, swapl, swaps, cpswapl, and cpswaps.
From deae18f3cad94959110761ef2ecd9665690e3db5 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 00:45:48 -0800 Subject: [PATCH 05/16] Xserver-spec: Fix assorted typos Signed-off-by: Alan Coopersmith Reviewed-by: Julien Cristau Reviewed-by: Jeremy Huddleston --- doc/xml/Xserver-spec.xml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/doc/xml/Xserver-spec.xml b/doc/xml/Xserver-spec.xml index f35dd0c63..74306373d 100644 --- a/doc/xml/Xserver-spec.xml +++ b/doc/xml/Xserver-spec.xml @@ -480,7 +480,7 @@ these operations. Before getting bogged down in the interface details, an typical usage example should establish the framework. Let's look at the ClientStateCallback in dix/dispatch.c. The purpose of this particular -callback is to notify intereseted parties when a client's state +callback is to notify interested parties when a client's state (initial, running, gone) changes. The callback is "created" in this case by simply declaring a variable:
@@ -489,7 +489,7 @@ case by simply declaring a variable: Whenever the client's state changes, the following code appears, which notifies -all intereseted parties of the change: +all interested parties of the change:
if (ClientStateCallback) CallCallbacks(&ClientStateCallback, (pointer)client);
@@ -759,7 +759,7 @@ These registered block handlers are called after the per-screen handlers: void (*BlockHandler) (blockData, pptv, pReadmask) pointer blockData; - OSTimePtr pptv; + OsTimerPtr pptv; pointer pReadmask;
@@ -770,7 +770,7 @@ which on UNIX family systems is generally represented by a struct timeval consisting of seconds and microseconds in 32 bit values. As a convenience to reduce error prone struct timeval computations which require modulus arithmetic and correct overflow behavior in the face of -millisecond wrapping throrugh 32 bits, +millisecond wrapping through 32 bits,
void AdjustWaitForDelay(pointer /*waitTime*, unsigned long /* newdelay */) @@ -875,7 +875,7 @@ and RemoveEnabledDevice are in Xserver/os/connection.c. Similarly, the X server or an extension may need to wait for some timeout. Early X releases implemented this functionality using block and wakeup handlers, but this has been rewritten to use a general timer facilty, and the -internal screen saver facilties reimplemented to use Timers. +internal screen saver facilities reimplemented to use Timers. These functions are TimerInit, TimerForce, TimerSet, TimerCheck, TimerCancel, and TimerFree, as defined in Xserver/include/os.h. A callback function will be called when the timer fires, along with the current time, and a user provided argument. @@ -913,11 +913,11 @@ for the timer entry. void TimerCancel(OsTimerPtr /* pTimer */) - void TimerFree(OSTimerPtr /* pTimer */) + void TimerFree(OsTimerPtr /* pTimer */)
-TimerInit frees any exisiting timer entries. TimerForce forces a call to the timer's +TimerInit frees any existing timer entries. TimerForce forces a call to the timer's callback function and returns true if the timer entry existed, else it returns false and does not call the callback function. TimerCancel will cancel the specified timer. TimerFree calls TimerCancel and frees the specified timer. @@ -1807,7 +1807,7 @@ printed on each keycap. (See X11/keysym.h) Legal modifier keys must generate both up and down transitions. When a client tries to change a modifier key (for instance, to make "A" the -"Control" key), DIX calls the following routine, which should retuurn +"Control" key), DIX calls the following routine, which should return TRUE if the key can be used as a modifier on the given device:
@@ -2708,7 +2708,7 @@ Xserver/dix/colormap.c.)
-ListInstalledColormaps fills the pCMapList in with the resource ids +ListInstalledColormaps fills the pCmapList in with the resource ids of the installed maps and returns a count of installed maps. pCmapList will point to an array of size MaxInstalledMaps that was allocated by the caller. @@ -3606,7 +3606,7 @@ this screen function. The new border width is given by width. This function is called for windows that are being unrealized as part of an UnrealizeTree. pChild is the window being unrealized, pWin is an -ancestor, and the fromConfigure value is simply propogated from UnrealizeTree. +ancestor, and the fromConfigure value is simply propagated from UnrealizeTree. @@ -5011,7 +5011,7 @@ mi and fb implementations. ListInstalledColormapsddxScreen LookupKeyboardDevicedix LookupPointerDevicedix -ModifyPixmapheadermiScreen +ModifyPixmapHeadermiScreen NextAvailableClientdix OsInitos PaintWindowBackgroundmiWindow From c1e769250078cbc74d7e6e6ddc5323c4f420ab14 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 08:06:40 -0800 Subject: [PATCH 06/16] Xserver-spec: Update location of log functions Signed-off-by: Alan Coopersmith Reviewed-by: Julien Cristau Reviewed-by: Jeremy Huddleston --- doc/xml/Xserver-spec.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/xml/Xserver-spec.xml b/doc/xml/Xserver-spec.xml index 74306373d..391ecb9a2 100644 --- a/doc/xml/Xserver-spec.xml +++ b/doc/xml/Xserver-spec.xml @@ -1344,7 +1344,7 @@ terminate the server; it must not return. The sample server implementation for these routines -is in Xserver/os/util.c. +is in Xserver/os/log.c along with other routines for logging messages. From 92cde0b84cdec164cd698dc74a7512d0791d7708 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 08:15:52 -0800 Subject: [PATCH 07/16] Xserver-spec: Update discussion of font library Signed-off-by: Alan Coopersmith Reviewed-by: Julien Cristau Reviewed-by: Jeremy Huddleston --- doc/xml/Xserver-spec.xml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/doc/xml/Xserver-spec.xml b/doc/xml/Xserver-spec.xml index 391ecb9a2..a2aec2f0f 100644 --- a/doc/xml/Xserver-spec.xml +++ b/doc/xml/Xserver-spec.xml @@ -1182,7 +1182,8 @@ are requests in that client's input queue. Font Support In the sample server, fonts are encoded in disk files or fetched from the -font server. +font server. The two fonts required by the server, fixed +and cursor are commonly compiled into the font library. For disk fonts, there is one file per font, with a file name like "fixed.pcf". Font server fonts are read over the network using the X Font Server Protocol. The disk directories containing disk fonts and @@ -1196,9 +1197,10 @@ appropriate code in the Font Library, you will automatically export fonts in that format both through the X server and the Font server. -With the incorporation of font-server based fonts and the Speedo donation -from Bitstream, the font interfaces have been moved into a separate -library, now called the Font Library (../fonts/lib). These routines are +The code for processing fonts in different formats, as well as handling the +metadata files for them on disk (such as fonts.dir) is +located in the libXfont library, which is provided as a separately compiled +module. These routines are shared between the X server and the Font server, so instead of this document specifying what you must implement, simply refer to the font library interface specification for the details. All of the interface code to the Font From ccbba444b7b8e1ba555532a847377600bea43d03 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sun, 28 Nov 2010 10:45:17 -0800 Subject: [PATCH 08/16] config: Remove AC_PROG_CC, SED & INSTALL that XORG_DEFAULT_OPTIONS provide Most importantly removes AC_PROG_CC call that resets compiler flags back to C89 mode, breaking use of C99 isfinite() on Solaris in dix/devices.c. Signed-off-by: Alan Coopersmith Reviewed-by: Gaetan Nadon --- configure.ac | 3 --- 1 file changed, 3 deletions(-) diff --git a/configure.ac b/configure.ac index 2363e3871..4365f5da7 100644 --- a/configure.ac +++ b/configure.ac @@ -64,9 +64,7 @@ dnl version-config.h covers the version numbers so they can be bumped without dnl forcing an entire recompile.x AC_CONFIG_HEADERS(include/version-config.h) -AC_PROG_CC AM_PROG_AS -AC_PROG_INSTALL AC_PROG_LN_S AC_LIBTOOL_WIN32_DLL AC_DISABLE_STATIC @@ -77,7 +75,6 @@ AC_PROG_LEX AC_PROG_YACC AC_SYS_LARGEFILE XORG_PROG_RAWCPP -AC_PROG_SED # Quoted so that make will expand $(CWARNFLAGS) in makefiles to allow # easier overrides at build time. From d75777d54c2107163305f50e8ee4306da202b95e Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sun, 28 Nov 2010 13:45:40 -0800 Subject: [PATCH 09/16] Move xchomp inside #ifdef __linux__ static function only called from the matchDriverFromFiles function that's inside #ifdef __linux__ section Signed-off-by: Alan Coopersmith Reviewed-by: Matt Turner --- hw/xfree86/common/xf86pciBus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c index 3e9c4551f..d6898322e 100644 --- a/hw/xfree86/common/xf86pciBus.c +++ b/hw/xfree86/common/xf86pciBus.c @@ -1167,6 +1167,7 @@ videoPtrToDriverList(struct pci_device *dev, return i; /* Number of entries added */ } +#ifdef __linux__ static int xchomp(char *line) { @@ -1183,7 +1184,6 @@ xchomp(char *line) return 0; } -#ifdef __linux__ /* This function is used to provide a workaround for binary drivers that * don't export their PCI ID's properly. If distros don't end up using this * feature it can and should be removed because the symbol-based resolution From d346bc3083c6d4bea59b77f634c7c5ec6c1d8cc9 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sun, 28 Nov 2010 13:48:16 -0800 Subject: [PATCH 10/16] Fix compiler warnings in hw/xfree86/os-support/solaris sun_init.c: In function `xf86OpenConsole': sun_init.c:99: warning: cast does not match function type sun_init.c:74: warning: unused variable `FreeVTslot' sun_init.c: In function `xf86UseMsg': sun_init.c:417: warning: old-style parameter declaration sun_vid.c: In function `solUnMapVidMem': sun_vid.c:162: warning: long unsigned int format, pointer arg (arg 6) sun_vid.c: In function `xf86ReadBIOS': sun_vid.c:217: warning: long unsigned int format, pointer arg (arg 5) sun_vid.c:217: warning: long unsigned int format, int arg (arg 6) sun_agp.c: In function `xf86EnableAGP': sun_agp.c:321: warning: unsigned int format, CARD32 arg (arg 4) Signed-off-by: Alan Coopersmith Reviewed-by: Julien Cristau --- hw/xfree86/os-support/solaris/sun_agp.c | 2 +- hw/xfree86/os-support/solaris/sun_init.c | 7 +++---- hw/xfree86/os-support/solaris/sun_vid.c | 5 ++--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/hw/xfree86/os-support/solaris/sun_agp.c b/hw/xfree86/os-support/solaris/sun_agp.c index 9db5d6368..dd4b1e2f9 100644 --- a/hw/xfree86/os-support/solaris/sun_agp.c +++ b/hw/xfree86/os-support/solaris/sun_agp.c @@ -318,7 +318,7 @@ xf86EnableAGP(int screenNum, CARD32 mode) if (ioctl(gartFd, AGPIOC_SETUP, &setup) != 0) { xf86DrvMsg(screenNum, X_WARNING, "xf86EnableAGP: " "AGPIOC_SETUP with mode %x failed (%s)\n", - mode, strerror(errno)); + (unsigned int) mode, strerror(errno)); return FALSE; } diff --git a/hw/xfree86/os-support/solaris/sun_init.c b/hw/xfree86/os-support/solaris/sun_init.c index edcc60b88..281a6df9f 100644 --- a/hw/xfree86/os-support/solaris/sun_init.c +++ b/hw/xfree86/os-support/solaris/sun_init.c @@ -71,7 +71,6 @@ xf86OpenConsole(void) int fd; struct vt_mode VT; struct vt_stat vtinfo; - int FreeVTslot; MessageType from = X_PROBED; #endif @@ -95,8 +94,8 @@ xf86OpenConsole(void) } else { - if ((int)mmap(0, 0x1000, PROT_NONE, - MAP_FIXED | MAP_SHARED, fd, 0) == -1) + if (mmap(0, 0x1000, PROT_NONE, + MAP_FIXED | MAP_SHARED, fd, 0) == MAP_FAILED) xf86Msg(X_WARNING, "xf86OpenConsole: failed to protect page 0 (%s)\n", strerror(errno)); @@ -413,7 +412,7 @@ xf86ProcessArgument(int argc, char **argv, int i) return 0; } -void xf86UseMsg() +void xf86UseMsg(void) { #ifdef HAS_USL_VTS ErrorF("vtX Use the specified VT number\n"); diff --git a/hw/xfree86/os-support/solaris/sun_vid.c b/hw/xfree86/os-support/solaris/sun_vid.c index 5089ae74d..94979736d 100644 --- a/hw/xfree86/os-support/solaris/sun_vid.c +++ b/hw/xfree86/os-support/solaris/sun_vid.c @@ -157,7 +157,7 @@ solUnMapVidMem(int ScreenNum, pointer Base, unsigned long Size) if (munmap(Base, Size) != 0) { xf86DrvMsgVerb(ScreenNum, X_WARNING, 0, "solUnMapVidMem: failed to unmap %s" - " (0x%08lx,0x%lx) (%s)\n", + " (0x%p,0x%lx) (%s)\n", apertureDevName, Base, Size, strerror(errno)); } @@ -212,8 +212,7 @@ xf86ReadBIOS(unsigned long Base, unsigned long Offset, unsigned char *Buf, (void)memcpy(Buf, (void *)(ptr + Offset), Len); if (munmap((caddr_t)ptr, mlen) != 0) { xf86MsgVerb(X_WARNING, 0, - "solUnMapVidMem: failed to unmap %s" - " (0x%08lx,0x%lx) (%s)\n", + "xf86ReadBIOS: failed to unmap %s (0x%p,0x%x) (%s)\n", apertureDevName, ptr, mlen, strerror(errno)); } From 29e467a1f1548a826ee2793244e3ff416aa1a0f2 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 26 Nov 2010 16:53:40 -0800 Subject: [PATCH 11/16] xf86OutputRename: Replace another strlen/malloc/strcpy set with strdup Signed-off-by: Alan Coopersmith Reviewed-by: Matt Turner --- hw/xfree86/modes/xf86Crtc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c index 7fc2a60f7..ae5aad38c 100644 --- a/hw/xfree86/modes/xf86Crtc.c +++ b/hw/xfree86/modes/xf86Crtc.c @@ -660,13 +660,11 @@ xf86OutputCreate (ScrnInfoPtr scrn, Bool xf86OutputRename (xf86OutputPtr output, const char *name) { - int len = strlen(name) + 1; - char *newname = malloc(len); + char *newname = strdup(name); if (!newname) return FALSE; /* so sorry... */ - strcpy (newname, name); if (output->name && output->name != (char *) (output + 1)) free(output->name); output->name = newname; From 2c8e534c8e9334562485aeaaef374871cf14d5fe Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 23:49:55 -0800 Subject: [PATCH 12/16] xf86ValidateModes: xnfalloc(strlen) + strcpy => xnfstrdup Signed-off-by: Alan Coopersmith Reviewed-by: Mikhail Gusarov Reviewed-by: Julien Cristau --- hw/xfree86/common/xf86Mode.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/xfree86/common/xf86Mode.c b/hw/xfree86/common/xf86Mode.c index 54fe0210d..644e5ce80 100644 --- a/hw/xfree86/common/xf86Mode.c +++ b/hw/xfree86/common/xf86Mode.c @@ -1643,8 +1643,7 @@ xf86ValidateModes(ScrnInfoPtr scrp, DisplayModePtr availModes, new = xnfcalloc(1, sizeof(DisplayModeRec)); new->prev = last; new->type = M_T_USERDEF; - new->name = xnfalloc(strlen(modeNames[i]) + 1); - strcpy(new->name, modeNames[i]); + new->name = xnfstrdup(modeNames[i]); if (new->prev) new->prev->next = new; *endp = last = new; @@ -1716,10 +1715,9 @@ xf86ValidateModes(ScrnInfoPtr scrp, DisplayModePtr availModes, p = xnfcalloc(1, sizeof(DisplayModeRec)); p->prev = last; - p->name = xnfalloc(strlen(r->name) + 1); + p->name = xnfstrdup(r->name); if (!userModes) p->type = M_T_USERDEF; - strcpy(p->name, r->name); if (p->prev) p->prev->next = p; *endp = last = p; From 4bbc90cd8b7e749fd8072ce7cd8dd998f4396981 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 19:06:56 -0800 Subject: [PATCH 13/16] xf86AutoConfig: make copyScreen memory allocation & error handling more sane No point calling the no-fail-alloc if you check for failure and your only caller checks for failure. No point calling calloc to zero fill memory you're about to memcpy over. In the unlikely event of a loss of memory allocation, drop your previous allocations before returning to others. Signed-off-by: Alan Coopersmith Reviewed-by: Julien Cristau Reviewed-by: Mikhail Gusarov --- hw/xfree86/common/xf86AutoConfig.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c index 200cb8f03..3cd5ef6ff 100644 --- a/hw/xfree86/common/xf86AutoConfig.c +++ b/hw/xfree86/common/xf86AutoConfig.c @@ -282,21 +282,31 @@ listPossibleVideoDrivers(char *matches[], int nmatches) static Bool copyScreen(confScreenPtr oscreen, GDevPtr odev, int i, char *driver) { + confScreenPtr nscreen; GDevPtr cptr = NULL; - xf86ConfigLayout.screens[i].screen = xnfcalloc(1, sizeof(confScreenRec)); - if(!xf86ConfigLayout.screens[i].screen) + nscreen = malloc(sizeof(confScreenRec)); + if (!nscreen) return FALSE; - memcpy(xf86ConfigLayout.screens[i].screen, oscreen, sizeof(confScreenRec)); + memcpy(nscreen, oscreen, sizeof(confScreenRec)); - cptr = calloc(1, sizeof(GDevRec)); - if (!cptr) + cptr = malloc(sizeof(GDevRec)); + if (!cptr) { + free(nscreen); return FALSE; + } memcpy(cptr, odev, sizeof(GDevRec)); cptr->identifier = Xprintf("Autoconfigured Video Device %s", driver); + if (!cptr->identifier) { + free(cptr); + free(nscreen); + return FALSE; + } cptr->driver = driver; + xf86ConfigLayout.screens[i].screen = nscreen; + /* now associate the new driver entry with the new screen entry */ xf86ConfigLayout.screens[i].screen->device = cptr; cptr->myScreenSection = xf86ConfigLayout.screens[i].screen; From 40d5a019352fa8f12230c863e11cbb1f6258a93e Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 19:50:38 -0800 Subject: [PATCH 14/16] xf86VIDrvMsgVerb: print args, not format string Signed-off-by: Alan Coopersmith Reviewed-by: Mikhail Gusarov Reviewed-by: Julien Cristau --- hw/xfree86/common/xf86Helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c index 78e6b200d..ea0acbfe4 100644 --- a/hw/xfree86/common/xf86Helper.c +++ b/hw/xfree86/common/xf86Helper.c @@ -1190,7 +1190,7 @@ xf86VIDrvMsgVerb(InputInfoPtr dev, MessageType type, int verb, const char *forma char *msg; msg = Xprintf("%s: %s: %s", dev->drv->driverName, dev->name, format); - LogVMessageVerb(type, verb, "%s", msg); + LogVMessageVerb(type, verb, msg, args); free(msg); } From 685286b17d30335d799a9da11914943e466ea955 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 20:43:28 -0800 Subject: [PATCH 15/16] FindModuleInSubdir: Stop allocating one more byte than needed 15ac25627e7239629be59 removed the "/" from the sprintf strings, but failed to remove the extra byte allocated for the '/'. Signed-off-by: Alan Coopersmith Reviewed-by: Mikhail Gusarov Reviewed-by: Julien Cristau --- hw/xfree86/loader/loadmod.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c index 6e6522774..06d082b34 100644 --- a/hw/xfree86/loader/loadmod.c +++ b/hw/xfree86/loader/loadmod.c @@ -406,21 +406,21 @@ FindModuleInSubdir(const char *dirpath, const char *module) snprintf(tmpBuf, PATH_MAX, "lib%s.so", module); if (strcmp(direntry->d_name, tmpBuf) == 0) { - ret = malloc(strlen(tmpBuf) + strlen(dirpath) + 2); + ret = malloc(strlen(tmpBuf) + strlen(dirpath) + 1); sprintf(ret, "%s%s", dirpath, tmpBuf); break; } snprintf(tmpBuf, PATH_MAX, "%s_drv.so", module); if (strcmp(direntry->d_name, tmpBuf) == 0) { - ret = malloc(strlen(tmpBuf) + strlen(dirpath) + 2); + ret = malloc(strlen(tmpBuf) + strlen(dirpath) + 1); sprintf(ret, "%s%s", dirpath, tmpBuf); break; } snprintf(tmpBuf, PATH_MAX, "%s.so", module); if (strcmp(direntry->d_name, tmpBuf) == 0) { - ret = malloc(strlen(tmpBuf) + strlen(dirpath) + 2); + ret = malloc(strlen(tmpBuf) + strlen(dirpath) + 1); sprintf(ret, "%s%s", dirpath, tmpBuf); break; } From 8f42b2b69387b006bfcd373c3d023ebea9035db2 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 27 Nov 2010 22:34:57 -0800 Subject: [PATCH 16/16] Simplify Error() - don't allocate temporary copy of error string Doesn't seem to be any reason to just not pass the error string as another argument directly to LogVWrite() Signed-off-by: Alan Coopersmith Reviewed-by: Mikhail Gusarov Reviewed-by: Julien Cristau --- include/os.h | 2 +- os/log.c | 17 +++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/include/os.h b/include/os.h index e882a0cf6..566514d4a 100644 --- a/include/os.h +++ b/include/os.h @@ -526,7 +526,7 @@ extern _X_EXPORT void FatalError(const char *f, ...) _X_ATTRIBUTE_PRINTF(1,2) _X extern _X_EXPORT void VErrorF(const char *f, va_list args); extern _X_EXPORT void ErrorF(const char *f, ...) _X_ATTRIBUTE_PRINTF(1,2); -extern _X_EXPORT void Error(char *str); +extern _X_EXPORT void Error(const char *str); extern _X_EXPORT void LogPrintMarkers(void); extern _X_EXPORT void xorg_backtrace(void); diff --git a/os/log.c b/os/log.c index d77708ea6..fdcf91c31 100644 --- a/os/log.c +++ b/os/log.c @@ -571,21 +571,14 @@ ErrorF(const char * f, ...) /* A perror() workalike. */ void -Error(char *str) +Error(const char *str) { - char *err = NULL; - int saveErrno = errno; + const char *err = strerror(errno); - if (str) { - err = malloc(strlen(strerror(saveErrno)) + strlen(str) + 2 + 1); - if (!err) - return; - sprintf(err, "%s: ", str); - strcat(err, strerror(saveErrno)); + if (str) + LogWrite(-1, "%s: %s", str, err); + else LogWrite(-1, "%s", err); - free(err); - } else - LogWrite(-1, "%s", strerror(saveErrno)); } void