Re: obj_coff_loc Internal Error

[Nick Clifton <nickc@cambridge.redhat.com>]

Hi Tracy,

> > First of all - do you have a binutils copyright assignment on file
> > with the FSF ?  Without such an assignment we cannot accept your
> > patch.  (I know that the patch is small, but I consider it to be
> > non-trivial and not-obvious, so we do need the assignment).
> 
> I am working on this.  I have yet to receive the form from FSF.

Is this the form you fill out to request copyright assignment ?  I can
send you a copy directly if you wish.


> > Secondly I am not sure that it is correct to issue a warning if the
> > comma is missing.  The code before your patch parsed the expression
> > without requiring an line number and I feel that we should continue to
> > support that behaviour unless there is a good reason not too.
> 
> We can remove the warning, but I thought it strange for the
> directive to accept two arguments that did not require a comma.

True, but it is possible that there are assemblers out there that
support this syntax.  Certainly this is what is implied by the
current code.

> > This does look wrong, or at least different.  The listing code does
> > change the variable 'lineno' which is used by the call to add_lineno()
> > so the behaviour will change.  Why did you move the call ?  Were you
> > detecting incorrect line numbers in your output ?
> 
> I could not find any documentation on this directive except for the
> comment block before the directive.
> 
> /* .loc is essentially the same as .ln; parse it for assembler
>    compatibility.  */
> 
> I made an assumption that if it were like .ln, then we would want it to
> perform like .ln.  Maybe "essentially" means that it actually acts
> differently (?)

It could :-)  Operating on the principle of "if ain't broke, don;t fix
it" however, I would suggest leaving the current code ordering alone
until such time as someone can prove that it is wrong.

> > Finally, since you are changing the behaviour of the .loc directive, it
> > would be great if you could add some documentation about it to the
> > assembler doc file (gas/doc/as.texinfo).  This is optional, but nice.
> 
> I do not see any documentation for this directive in gas/doc/as.texinfo.
> I wanted to make sure I understood the directive (and was suggesting the
> correct patch) before adding any documentation.

Fair enough.  Of course by writing the documentation you also get to
define the behaviour. :-)  Actually the lack of documentation on this
and similar directives was why I suggested that you write something in
the first place.  If you document this directive you, or someone else
might be motivated to document the others, which in my opinion at
least, would be a good thing.

Cheers
        Nick