[PATCH 5/7] xf86ShowOpts.c: Remove bad code from DoShowOptions

Alan Coopersmith alan.coopersmith at oracle.com
Wed Nov 2 17:56:34 PDT 2011


On 11/01/11 19:08, Daniel Stone wrote:
> Hi,
>
> On 2 November 2011 01:14, Alan Coopersmith<alan.coopersmith at oracle.com>  wrote:
>> On 11/01/11 17:50, Daniel Stone wrote:
>>> On 1 November 2011 22:42, Alan Coopersmith<alan.coopersmith at oracle.com>
>>>   wrote:
>>>> When we want to print a string, it's okay to just print it.
>>>> We don't need to first allocate a buffer 2 bytes bigger than the
>>>> string, copy the entire string unmodified to the buffer, print the
>>>> buffer, and then leak the buffer (though we AbortDDX 8 lines later,
>>>> and then just in case we survived that, call exit as well, so the
>>>> leak is short lived, just oh so pointless).
>>>>
>>>> Oh, and for good measure, put the "r" in String, no matter how
>>>> much it stings.
>>>
>>> Bonus points if you felt like just removing optionTypeToSting, now
>>> that it's unused.
>>
>> It's still used - I just put the call directly in the ErrorF
>> argument, instead of storing in a variable:
>>
>> +                                       ErrorF ("\t%s:%s\n", p->name,
>> +
>> optionTypeToString(p->type));
>
> Sure, that's optionTypeToSt_R_ing -- optionTypeToSting is now a sad
> unwanted orphan.  (Yes, there were two functions named mostly the same
> thing, doing mostly the same thing.)

Ah, I hadn't noticed the duplication/overlap/insanity.

Actually, it looks like they're both used still, since there's two static
copies, one in hw/xfree86/common/xf86ShowOpts.c (the Sting one I renamed
in this patch) and one in hw/xfree86/common/xf86Configure.c, and of course,
they output slightly different strings for certain values.

I could change the hw/xfree86/common/xf86Configure.c one to be _X_HIDDEN
instead of static, so we can call it from xf86ShowOpts.c, or I could just
move the single useful function from xf86ShowOpts.c into xf86Configure.c
so it can call the existing static copy.

Merging the files seems slightly saner to me than exporting the helper
function - anyone disagree?

-- 
	-Alan Coopersmith-        alan.coopersmith at oracle.com
	 Oracle Solaris Platform Engineering: X Window System



More information about the xorg-devel mailing list