Hi Nick, thanks for chipping in,

Sorry for dragging you into this, it's just that I thought you were the
best person to look to since you authored the .seh_code directive some time
(9 years!!) ago :P

0. I don't mind submitting this under whatever license, so long as I can
contribute it, I'm happy

1. Ah, I thought I could get away with that warning, I didn't know binutils
was compiled with -Werror on. I'll fix it, thanks for the heads up

2. I'm not too sure how to check if there are more than 4, I'll get back to
you on that

3. Where should the documentation be? I know what some (but not all) of the
directives do, so I'll be happy to document the ones I know

4. Ah, the joys of auto formatting. I'll try to get rid of all of them in
the final patch

Thanks for getting back to me, I appreciate the time! :)

best regards,
Julian

On Wed, Jul 19, 2023 at 6:53 PM Nick Clifton <nickc@redhat.com> wrote:

> Hi Julian,
>
> > Anyone? Some reviews would help, maybe Nick or Alan?
>
> Sorry for the delay in reviewing your patch.
>
> Unfortunately there are a few problems:
>
> 0.  Do you have an FSF Copyright assignment for contributions
>      to the GNU Binutils project.  Or are you willing to submit
>      the patch under the terms of a Developer's Certificate of
>      Origin ?
>
> 1. The patch does not compile:
>
> In file included from gas/config/obj-coff.c:57:
> gas/config/obj-coff-seh.c: In function 'obj_coff_seh_scope':
> gas/config/obj-coff-seh.c:406:29: error: comparison between pointer and
> integer [-Werror]
>    406 |     if (*input_line_pointer == NULL || *input_line_pointer ==
> '\n') {
>        |                             ^^
>
> 2. The code does not check to see if more than 4 arguments have been
>     supplied to the .seh_scope directive.
>
> 3. There ought to be some description of the .seh_scope directive in
>     the assembler's documentation.   (Although it appears that none
>     of the directives are documented, which is naughty).  If your patch
>     were to add some then this might encourage others to extend the
>     coverage to the other directives.
>
> 4. There are various (minor) formatting issues that should be fixed
>     such as the indentation, unnecessary blank lines, long lines and
>     so on.  Please have a look at the GNU Coding Standards for more
>     information:  https://www.gnu.org/prep/standards/
>
> Cheers
>    Nick
>
>