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

Bug#342659: Found the problem



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.

Sjoerd Hemminga
Snow B.V.



Reply to: