[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#342659: Found the problem



On Fri, Dec 09, 2005 at 03:41:23PM +0100, Sjoerd Hemminga wrote:
> The problem is that 028_loader_speed_hack.diff optimizes too much :)
> 
> In xc/programs/Xserver/hw/xfree86/loader/loadmod.c, function
> LoaderListDirs(), your patch removed all work on fp before
> fp = LoaderGetCanonicalName(buf, NULL);
> This line is added by your patch.
> 
> However in the upstream source fp points _into_ buf, so changes to fp,
> change buf. Directly before the aforementioned line, a test is done to check
> whether buf points to a regular file or not. Since the apropriate changes to
> buf won't be made anymore, this test fails because buf points to a
> directory, not to a file.
> 
> Due to this failure, the return variable (listing) will remain NULL
> throughout the whole function. LoaderListDirs() is therefore reduced to a
> quite expensive return NULL :)
> 
> Seeing that this function will never be able to work properly and that X.org
> remains perfectly functional - with the only noted exception being the
> probing and autoconfiguration I reported before - one might conclude that
> the function is not called very often. From this, one might also conclude
> that optimizations in it are a waste of (valuable) programmer time and a bug
> hazard.
> 
> I think a proper fix would be to reinstate the following three lines:
> 
> After line 437 [ strcat(buf, *s); ]:
> fp = buf + dirlen;
> 
> After line 440 [ buf[dirlen++] = '/'; ]:
> fp++;
> 
> After line 444 [ continue; ]:
> strcpy(fp, dp->d_name);
> 
> How three lines can keep a guy busy for 16 hours truly amazes me.

LoaderListDirs is only really used by Xorg -configure, and this all got
properly fixed in CVS HEAD.  Speaking as the guy who wrote #028, I
didn't care about LLD().



Reply to: