[PATCH 1/2] Ensure all resource types created have names registered

[Alan Coopersmith]

Keith Packard wrote:
> On Thu, 10 Dec 2009 22:53:48 -0800, Alan Coopersmith <alan.coopersmith at sun.com> wrote:
> 
>>      MultibufferDrawableResType =
>> -	CreateNewResourceType(MultibufferDrawableDelete)|RC_DRAWABLE;
>> +	CreateNewResourceType(MultibufferDrawableDelete);
>>      MultibufferResType = CreateNewResourceType(MultibufferDelete);
>>      MultibuffersResType = CreateNewResourceType(MultibuffersDelete);
>>      OtherClientResType = CreateNewResourceType(OtherClientDelete);
>> @@ -479,6 +481,12 @@ MultibufferExtensionInit()
>>  				 ProcMultibufferDispatch, SProcMultibufferDispatch,
>>  				 MultibufferResetProc, StandardMinorOpcode)))
>>      {
>> +	MultibufferDrawableResType |= RC_DRAWABLE;
>> +	RegisterResourceName(MultibufferDrawableResType,
>> +			     "MultibufferDrawable");
>> +	RegisterResourceName(MultibufferResType, "MultibufferBuffer");
>> +	RegisterResourceName(MultibuffersResType, "MultibufferWindow");
>> +	RegisterResourceName(OtherClientResType,
>> "MultibufferOtherClient");
> 
> Where do you want to add RC_DRAWABLE to MultibufferDrawableResType?
> Twice is probably not correct.

I'm not understanding - the first hunk removes the original place it
was set, the second hunk adds a new setting.

>>  	XRC_DRAWABLE = CreateNewResourceClass();
>> -	XRT_WINDOW = CreateNewResourceType(XineramaDeleteResource) | 
>> -						XRC_DRAWABLE;
>> -	XRT_PIXMAP = CreateNewResourceType(XineramaDeleteResource) | 
>> -						XRC_DRAWABLE;
>> +	XRT_WINDOW = CreateNewResourceType(XineramaDeleteResource);
>> +	XRT_PIXMAP = CreateNewResourceType(XineramaDeleteResource);
>>  	XRT_GC = CreateNewResourceType(XineramaDeleteResource);
>>  	XRT_COLORMAP = CreateNewResourceType(XineramaDeleteResource);
>>  
>>  	panoramiXGeneration = serverGeneration;
>> -	success = TRUE;
>> +	if (XRT_WINDOW && XRT_PIXMAP && XRT_GC && XRT_COLORMAP) {
>> +	    RegisterResourceName(XRT_WINDOW, "XineramaWindow");
>> +	    RegisterResourceName(XRT_PIXMAP, "XineramaPixmap");
>> +	    RegisterResourceName(XRT_GC, "XineramaGC");
>> +	    RegisterResourceName(XRT_COLORMAP, "XineramaColormap");
>> +
>> +	    XRT_WINDOW |= XRC_DRAWABLE;
>> +	    XRT_PIXMAP |= XRC_DRAWABLE;
>> +	    success = TRUE;
>> +	}
> 
> Looks like RegisterResourceName should allow the resource type to come
> in with a class mask and should strip that off before setting the name,
> that way this patch would be much smaller.

RegisterResourceName already strips TypeMask - the move of the | XRC_DRAWABLE
is so that the check to see if CreateNewResourceType returned 0 isn't
defeated by setting the XRC_DRAWABLE bit before we've checked it for 0.
It's just a common bug I happened to notice while making this change.

>> >  	RT_INPUTCLIENT = CreateNewResourceType((DeleteType) InputClientGone);
> 
> Looking at this, I can't help but wonder if CreateNewResourceType (which
> should only be called at server startup time) shouldn't be using
> xnfalloc instead of having every caller check the return and call FatalError.

Most callers just fail the initialization of the extension - XInput is
special since it refuses to let the server run without the extension.

There's several cases that can cause CreateNewResourceType to return 0,
failure of it's realloc is just one - the others appear to be hitting
a clash with CreateNewResourceClass and failure of the realloc in
dixRegisterPrivateOffset().


>>  #include <X11/extensions/Xv.h>
>> @@ -1865,7 +1866,9 @@ void XineramifyXv(void)
>>  
>>     XvXRTPort = CreateNewResourceType(XineramaDeleteResource);
>>  
>> -   if(!xvsp0) return;
>> +   if (!xvsp0 || !XvXRTPort) return;
> 
> I'd like to see bug fixes separated out from the rest of the resource
> name fixing, unless you think that would be even more confusing.
> There are lots of other places that have pure bug fixes (checking for
> allocation failure) mixed in with the resource name fixes.

I can work on that.

[BTW, patch 2/2 was sent, but at 45kb got caught in the moderation queue.
 Should we raise the limit to be > 40kb, or leave it as a hint to break
 patches into smaller chunks when possible?]

-- 
	-Alan Coopersmith-           alan.coopersmith at sun.com
	 Sun Microsystems, Inc. - X Window System Engineering