This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patchv2] Support .dwp with the name of symlinked binary file


On Fri, 06 Sep 2013 20:29:09 +0200, Doug Evans wrote:
> On Thu, Sep 5, 2013 at 6:18 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> > Also currently GDB code seems to use bfd->filename and its objfile->name
> > interchangeably so I did not want to change either.
> 
> One needn't address things all at once, but not wanting to change
> something isn't, in and of itself(!), a good reason.

The posted patches
	[patchv3 3/5] Code cleanup: Add objfile_name accessor
	[patchv3 4/5] Keep objfile original filename

therefore:
 * unify the two separate strings into one (the bfd one, canonical one).
 * keep backward compatibility all the current code uses the bfd string.
 * preserve the non-canonical string for special cases
   (accessing explicitly objfile->original_name)


> Is there a real issue here?  I'm not sure, but it seems so.
> >From the point of view of the user, realpath bugs me.
> [I'm not including stripping "./" in this, stripping that is reasonable enough.
> And foo/bar/../baz isn't worth the trouble, though I would be ok with
> an option where the user can turn canonicalization on.]

I found the foo/bar/../baz paths unexpectedly annoying, after Pedro's
examples.


> ISTM realpath is mostly just an implementation detail in gdb.
> Or to turn it around, when *does* the user want realpath?
> If it is indeed rare that the user wants (or cares) about realpath,
> can we move towards not showing it to them?

ISTM people want the directories normalized against any "../".

I was giving a countercase where realpath gives worse result
	/home/user/file
->
	/.sdb5/home/user/file
but that happens rarely and the "../" normalization is a better benefit so
that one prefers to use gdb_realpath().


> It may be that there are places where we don't have anything else
> besides bfd->filename,
> but we already wrap bfd in gdb_bfd (I'm all for abstracting away bfd
> as much as possible).
> We *could* save the file passed to gdb_bfd_open,
> and have gdb_bfd_filename() or some such.

IIUC you were proposing thin 'struct gdb_bfd *' wrapper around 'bfd *'.

In the patchset above I have found we do not IMO need non-canonical filename
from 'bfd *', it is enough to have non-canonical filename from 'objfile *'.


> Also, it may be that not all objfiles have a bfd (I don't remember off
> hand, but whether it's true or not is irrelevant to my point), and if
> not all objfiles have a bfd we can't just remove objfile->name
> (assuming objfiles-without-bfds have a name).
> OTOH, if objfiles-without-bfds have a name, we could store it in a
> different place (objfile->foo_name), and thus still effectively have
> an implementation that only maintains one name, not two (setting aside
> bfd->filename which is more an implementation detail of the cache).
> [I don't want to add more file names, having two copies of something
> is already causing trouble.  But I think this is a solvable problem.]

See the top paragraph.


> And if we record the original file name we can always
> get the realpath name,

Calling gdb_realpath() dynamically is very expensive.  Fortunately bfd's
canonical filename can be used for it / as a cache.


Jan


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]