Re: [patch]: Support coff noread flag for segments

[Dave Korn <dave dot korn dot cygwin at googlemail dot com>]

Kai Tietz wrote:
> Hi,

    Hi Kai,

> As I added some time ago the NOREAD flag support to bfd, this is the
> final part for allowing the declaration of sections with
> SEC_COFF_NOREAD flag set. I used for this flag character 'y' (possibly
> another one is prefered?).
> 
> ChangeLog
> 
> 2009-05-15  Kai Tietz  <kai.tietz@onevision.com>
> 
> 	* config/obj-coff.c (obj_coff_section): Add 'y' as
> 	specifier for SEC_COFF_NOREAD section flag.
> 
> I tested x86_64-pc-mingw32 and i686-pc-cygwin.
> Is this patch ok for apply?

  Just a couple of very minor points:

@@ -1589,6 +1590,10 @@
 		case 'o': /* STYP_OVER */
 		  as_warn (_("unsupported section attribute '%c'"), attr);
 		  break;
+		case 'y':
+		  flags |= SEC_COFF_NOREAD;
+		  flags &= ~SEC_READONLY;
+		  break;

 		default:
 		  as_warn (_("unknown section attribute '%c'"), attr);

1.  I think you ought to be setting readonly_removed in this clause?
2.  There should be a blank line before the 'case' so it doesn't look like a
fall-through.
3.  While you're doing that anyway, why not move it above cases 'i', 'l' and
'o', so that all the functional case clauses are in one uninterrupted stretch
at the start of the switch and both the error cases are right at end?

  Ok once we're clear about the readonly_removed issue (and with a re-test if
I'm correct and we should be setting it).

    cheers,
      DaveK