Re: [RFC] Fix .text section offset for windows DLL (was Calling __stdcall functions in the inferior)

[Joel Brobecker <brobecker@adacore.com>]

>   Oh boy, you scared me there...  But definitively, no, I didn't
>   commit this change yet!

Sorry about the scare. I was looking at the code, and saw identical
code, so I figured it got checked in.  That's perhaps a clue for us
to do more code factoring...

>   I still didn't get a correct way of checking my formatting...

No problemo, I don't think it's a major issue.

> > > +	  printf ("%s is a forward to %s\n", funcname, forward_name);
> > 
> > I don't think a printf is appropriate, here. Is that meant to be
> > a warning?
> 
>   I would have like any suggestion about an idea about how to
>   implement such a forward...  Something like a function called
>   add_pe_forward_symbol Would it require the definition of a new
>   minimal symbol type?  If yes, I wouldn't even know where I should
>   add this new type...

Are you saying, remove the need for the printf/warning by implementing
the missing feature? If complicated, I'd start with the
warning/complaint first, get that checked in, and then think about
how to lift the limitation.

> I basically reused the existing code in the function called
> read_pe_exported_syms, and, specifically, I kept most of the local
> variables and used the same names...

Let's keep it consistent for now, then...

> > > +  unsigned char *expdata, *erva;
> > 
> > Should we be using gdb_byte *, in this case? I'm wondering if we
> > should be adjusting the pe_get* & pe_as* routines as well...
> 
>   The problem is that it is used both for 'char *'
> for all the names and for RVA (relative virtual addresses)
> which are more something like a 32-bit unsigned offset.

I'm not opposed to keeping the consistency, but I think we're going
to have to re-evaluate the use of "char *" vs "gdb_byte *" at some
point. I think that if you have a buffer of bytes, it's better to
use gdb_bytes, as the type was created to deal with this sort of
data. Usually, you can't rely on what the buffer contains, because
host and target can be different.

> > > +  if (!is_pe32 && !is_pe64)
> > > +    {
> > > +      /* This is not a recognized PE format file.  Abort now, because
> > > +	 the code is untested on anything else.  *FIXME* test on
> > > +	 further architectures and loosen or remove this test.  */
> > > +      return 0;
> > > +    }
> > 
> > I think a complaint would be appropriate, here. And I'm wondering
> > if there might be a better way to check which PE format it is other
> > than looking at a string...
>
>  Again, this is a plain copy of the code in read_pe_exported_syms.

OK, you can keep it that way, at least for now. "plain copy" -> factorize?

-- 
Joel