Page MenuHomeFreeBSD

Make GDB build when basename() is POSIX compliant.
ClosedPublic

Authored by ed on May 29 2016, 3:07 PM.

Details

Summary

GDB's libiberty.h has a prototype of the basename() function it which
does not match the one that is part of POSIX, declared in libgen.h. This
is normally never visible, as GDB never includes libgen.h. On FreeBSD,
it unfortunately is, as our locally added copy of kgdb includes both.

Fix up libiberty.h to just include libgen.h. I'm currently discussing
with upstream how a clean fix should be done, but I guess that
requires more refactoring to the existing code. We'd better not bother
importing that and stick to this compact workaround.

I'll also ask the port maintainers to look at this change.

Test Plan

The ports build on my patched up system.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ed updated this revision to Diff 17069.May 29 2016, 3:07 PM
ed retitled this revision from to Make GDB build when basename() is POSIX compliant..
ed updated this object.
ed edited the test plan for this revision. (Show Details)
ed added a reviewer: jhb.
ed added a reviewer: bapt.May 29 2016, 3:16 PM

It would seem, the "fall-back" declaration is only used if the Operating System's one is no detected by configure (!HAVE_DECL_BASENAME).

If we are fixing this anyway, maybe, we ought to fix the upstream's configure to properly detect and use the local <libgen.h> instead? That would seem cleaner and more likely to be accepted upstream too...

On a side note, I'd also support adding something like const char *cbasename(const char *path) -- whereas dirname inserts a '\0' and thus can not be said to operate on read-only argument, basename does not modify its argument. So, a variant, that returns a const string would be useful (and may ease the job of future portings).

This revision now requires changes to proceed.May 29 2016, 3:42 PM
ed added a comment.May 29 2016, 3:59 PM

Hi there,

It would seem, the "fall-back" declaration is only used if the Operating System's one is no detected by configure (!HAVE_DECL_BASENAME).

If we are fixing this anyway, maybe, we ought to fix the upstream's configure to properly detect and use the local <libgen.h> instead? That would seem cleaner and more likely to be accepted upstream too...

I understand the angle you're coming from, but I respectfully disagree. The question is, why should configure check for this to begin with? Why should this chunk of code exist anyway?

I'd be interested in knowing which non-ancient operating systems supported by GDB do not ship with a <libgen.h> that contains a usable basename(). I can assure you: the answer is going to be none. All of this code is just a left-over of something that was needed a decade ago.

This is why I'm only interested in writing a tiny patch for the copy of GDB we have in base and ports. It works. Upstream may well make more aggressive changes, either making the tests for finding a proper basename() even more convoluted, or doing the cleanup as I hinted at above.

Ok, fine, proceed then :)

But, if you are messing with the base <libgen.h> (and the corresponding function-definitions), give the idea of const-poisoning cbasename a consideration... Maybe, there is even a POSIX prototype for something like that already...

(BTW, I wish I could comment as myself (mi), but for some reason I could not login with the same credentials I use with Bugzilla...)

This revision is now accepted and ready to land.May 29 2016, 4:03 PM
bapt accepted this revision.May 29 2016, 10:39 PM
bapt edited edge metadata.
This revision was automatically updated to reflect the committed changes.