Re: Request for review: 7196576: javac warnings cleanup from Adopt OpenJDK bugathon

[Alex Menkov]

Hi Martijn,

Updated fix look fine to me

regards
Alex

On 25.09.2012 19:40, Martijn Verburg wrote:

Hi all,

Apologies for the delay.  I've updated the patch and sent it through
to Artem to update his webrev.

Thanks,
Martijn

On 18 September 2012 10:46, Alex Menkov <alexey.men...@oracle.com> wrote:

Hi Martijn,

Generally the fix looks good, but there are some notes:

src/share/classes/com/sun/media/sound/AlawCodec.java

   // set the AudioInputStream length in frames if we know it
   if (stream instanceof AudioInputStream) {
-     frameLength = ((AudioInputStream)stream).getFrameLength();
+     frameLength = stream.getFrameLength();
   }

stream is always instance of AudioInputStream and is not null
so the check for instanceof could be dropped:

- // set the AudioInputStream length in frames if we know it
- if (stream instanceof AudioInputStream) {
-     frameLength = ((AudioInputStream)stream).getFrameLength();
- }
+ frameLength = stream.getFrameLength();


src/share/classes/com/sun/media/sound/UlawCodec.java
  - the same note as for AlawCodec.java


src/share/classes/javax/sound/midi/MidiSystem.java:

   } catch (MidiUnavailableException e) {
       // something went wrong with synth
       if (e instanceof MidiUnavailableException) {
-         mue = (MidiUnavailableException) e;
+         mue = e;
       }
   }

analogically should be
   } catch (MidiUnavailableException e) {
       // something went wrong with synth
-      if (e instanceof MidiUnavailableException) {
-         mue = (MidiUnavailableException) e;
-      }
+      mue = e;
   }

and a bit more complex case several line later:
   // then try to connect to the default Receiver
   try {
       rec = MidiSystem.getReceiver();
   } catch (Exception e) {
       // something went wrong. Nothing to do then!
       if (e instanceof MidiUnavailableException) {
           mue = (MidiUnavailableException) e;
       }
   }

MidiSystem.getReceiver() may throw only MidiUnavailableException, so the
statements could be simplified to:
   // then try to connect to the default Receiver
   try {
       rec = MidiSystem.getReceiver();
   } catch (MidiUnavailableException e) {
       // something went wrong. Nothing to do then!
       mue = e;
   }


regards
Alex


On 15.09.2012 16:10, Martijn Verburg wrote:

Hi all,

Artem Ananiev very kindly raised bugs and a webrev for the patches
sent in from a Bugathon we ran back in April (patches have been tested
against latest source tree).

The bug numbers are:

7196576: [Bugathon] Reduce the number of javac warnings in JavaSound

The corresponding webrevs are at:

http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.media/

Thanks to Artem, Stuart and Phil for helping out with the correct
splitting of patches etc.

Cheers,
Martijn