I'm adding Weijun (Max) Wang to this thread.
The same "ackbarfaccept" code had come up a third time when I was reviewing
some of Max's changes. The code in question all has to do with permissions, and
Max is in the security group, so he might have a better insight whether doing a
refactoring is appropriate and how to approach doing it.
Some searching around reveals that "ackbarfaccept" appears in the
PlayerPermission class of the Java ME Mobile Media API (not in any of the
OpenJDK repositories) so it looks like the code has been copied even more times
than are visible here. Perhaps a public permissions parsing utility method is
called for.
s'marks
On 12/2/11 9:02 AM, Sebastian Sickelmann wrote:
Am 02.12.2011 16:27, schrieb Brandon Passanisi:
Hi Sebastian. I'm not sure if you had seen the e-mail from Stuart Marks
regarding this, but Stuart was able to find more instances of the similar
block of "fallthrough" code. I can volunteer to apply your upcoming change
to FilePermission to the other files if you wish. Or, you can try applying
the change to the other files, but if you don't have time I don't mind doing
it. Here's the section of Stuart's e-mail on this topic:
-------------------------------------------------------------------------------------
(Incidentally, this is the third time I've reviewed code today that
looks exactly like this. The other cases are in java.io.FilePermission
and java.util.PropertyPermission. They each have the /*FALLTHROUGH*/
into a set of cases that do nothing but break; and they have similar
("ackbarfaccept") comments. It would be nice if these chunks of code
could be unified, but they differ in a number of fiddly details.)
(The string "ackbarfaccept" occurs in the following files:
1. java/io/FilePermission.java
2. java/net/SocketPermission.java
3. java/util/PropertyPermission.java
4. javax/management/MBeanPermission.java
5. javax/security/auth/kerberos/ServicePermission.java
Hmmm.)
-------------------------------------------------------------------------------------
Thanks.
Hi Brandon,
Hi Stuart,
i had a look at all those classes and it seems to be the same algorithm. In an
normal project (not the jdk) i would suggest to completely refactor it and make
some code de-duplication cleanup. But if i thinks on projects like jigsaw this
can easily get a real problem. What do you think, should we try to cleanup them
all. Or should we try to make some de-duplication/code-reuse refactoring.
-- Sebastian
On 12/1/2011 10:18 PM, Sebastian Sickelmann wrote:
Hi Brandon,
i will try to work out a fix for both and cc the review request to you.
-- Sebastian
Am 01.12.2011 23:54, schrieb Brandon Passanisi:
Hi Sebastian. I was speaking with Stuart Marks earlier today and he
mentioned that the "fallthrough" code in FilePermission.java also exists in
java.util.PropertyPermission.java. Maybe the code author had done some
copy/paste when working on these files. Stuart had said that you might be
planning on doing some work on this after the warnings cleanup work.
If/when you do this work, maybe you can let me know so that I can sync the
same changes you apply to FilePermission.java to PropertyPermission.java?
Or, maybe you can apply the same changes yourself to PropertyPermission.java?
