review (S) for 6826261: class file dumping from SA is broken

[Tom Rodriguez]

That's a good idea.  How about this relative to the old one:

http://cr.openjdk.java.net/~never/6826261/edit/agent/src/share/classes/sun/jvm/hotspot/tools/jcore/ClassWriter.java.udiff.html

tom

On Apr 3, 2009, at 2:03 PM, John Coomes wrote:

> Tom Rodriguez (Thomas.Rodriguez at Sun.COM) wrote:
>> http://cr.openjdk.java.net/~never/6826261
>
> I'm a noob to the SA, but looks good to me.  One minor suggestion in
> ClassWriter.java.  The common pattern for checking for
> synthetic methods is
>
> 364             boolean isSyn = isSynthetic(accessFlags);
> 365             if (isSyn && _syntheticIndex != 0)
> 366                 fieldAttributeCount++;
>
> where you've added && _syntheticIndex != 0.  Can you fold this check
> into isSyn?  E.g.:
>
>  boolean isSyn = _syntheticIndex != 0 && isSynthetic(accessFlags);
>
> The uses of isSyn (without the check for _syntheticIndex != 0) simply
> guard a call to writeSynthetic(), and that does its own check for
> _syntheticIndex != 0.  Except for this use in writeMethod():
>
> 452         if (isSyn) {
> 453             if (DEBUG) debugMessage("\tmethod is synthetic");
> 454             writeSynthetic();
> 455         }
>
> Not sure if that debug message is still valid when _syntheticIndex ==
> 0, since writeSynthetic() won't write anything.
>
> -John
>