[rfc][icedtea-web] itw itself warning cleanup
Jiri Vanek
jvanek at redhat.com
Fri Dec 13 04:57:16 PST 2013
On 12/12/2013 06:10 PM, Pavel Tisnovsky wrote:
Thanx! pushed an backported to 1.4. Both indentation issues fixed. Unluckily the map is out of scope of this patch.
As you probably noted, I had to revert few (really minimum - http://icedtea.classpath.org/hg/icedtea-web/rev/48b7db08527c) changes to make it compatible with jdk6 (again).
I made this two changeset intentionaly - As I knew I would be on older system today (And so tune it) and as remiander for itw 1.6 or 1.7 to ghet rid of them :)
What do you think about other warnings?
warning: [deprecation] >> Ref in sun.misc has been deprecated
warning: [deprecation] >> toURL() in File has been deprecated
and proprietary api
- there is hardly something to do....?-(
But following should be fixed. I think that the _breaks_ should be added. But I'm not sure. It was added by Omair in 2011, do you remember who *reviewed* it?
What is your personal opinion on it?
icedtea-web/netx/net/sourceforge/jnlp/browser/BrowserAwareProxySelector.java:205: warning:
[fallthrough] possible fall-through into case
case BROWSER_PROXY_TYPE_SYSTEM:
^
icedtea-web/netx/net/sourceforge/jnlp/browser/BrowserAwareProxySelector.java:211: warning:
[fallthrough] possible fall-through into case
default:
^
which is:
@Override
protected List<Proxy> getFromBrowser(URI uri) {
List<Proxy> proxies = new ArrayList<Proxy>();
String optionDescription = null;
switch (browserProxyType) {
case BROWSER_PROXY_TYPE_PAC:
proxies.addAll(getFromBrowserPAC(uri));
break;
case BROWSER_PROXY_TYPE_MANUAL:
proxies.addAll(getFromBrowserConfiguration(uri));
break;
case BROWSER_PROXY_TYPE_NONE:
proxies.add(Proxy.NO_PROXY);
break;
case BROWSER_PROXY_TYPE_AUTO:
// firefox will do a whole lot of stuff to automagically
// figure out the right settings. gconf, WPAD, and ENV are used.
// https://bugzilla.mozilla.org/show_bug.cgi?id=66057#c32
// TODO this is probably not easy/quick to do. using libproxy might be
// the simpler workaround
if (optionDescription == null) {
optionDescription = "Automatic";
}
case BROWSER_PROXY_TYPE_SYSTEM:
// means use $http_proxy, $ftp_proxy etc.
// TODO implement env vars if possible
if (optionDescription == null) {
optionDescription = "System";
}
default:
if (optionDescription == null) {
optionDescription = "Unknown";
}
OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG,R("RProxyFirefoxOptionNotImplemented",
browserProxyType, optionDescription));
proxies.add(Proxy.NO_PROXY);
}
OutputController.getLogger().log("Browser selected proxies: " + proxies.toString());
return proxies;
}
Well the whole code smells:
The first if (optionDescription == null) is always true (or not!?!?!?) so it is always
optionDescription = "Automatic"; Then it bubble into BROWSER_PROXY_TYPE_SYSTEM, where it can not set
as it is already not null. So imho both breaks are really missing. But the TODO in comments make me
feel uncomfortable.
> Hi Jiri,
>
> this change looks good, some minor problems:
>
> + private void addComponents() {<- two spaces,
>
> + public static Class<?> primitiveNameToType(String name) {<- is not Map better here instead of those if-else chain?
>
> Wrong indentation:
> + if (ret instanceof Throwable) {
> + throw (Throwable) ret;
> + }
>
> Thank you,
> Pavel
>
> ----- Jiri Vanek<jvanek at redhat.com> wrote:
>> On 12/12/2013 12:30 PM, Jiri Vanek wrote:
>>> After this clean up only "internal proprietary API and may be removed in a future release" warnings
>>> remain fro make check.
>>>
>>>
>>> IcedTea-Web itself is another in queue as warnings have really multiplied.
>>>
>>>
>>> I would like to backport as much of this clceanup to 1.4 as possible.
>>>
>>> J.
>> Here we go:
>>
>>
>> Fixed:
>> rawtype, uncheck, braces, and Override annotations
>>
>> Again, I would like to backport as much as posible to 1.4.
>>
>>
>> subjects of "future patches" (but included here)
>> icedtea-web/netx/net/sourceforge/jnlp/util/logging/ConsoleOutputPane.java:248: warning: [rawtypes]
>> found raw type: JComboBox
>> icedtea-web/netx/net/sourceforge/jnlp/util/logging/ConsoleOutputPane.java:337: warning: [rawtypes]
>> found raw type: DefaultComboBoxModel
>> icedtea-web/netx/net/sourceforge/jnlp/util/logging/ConsoleOutputPane.java:827: warning: [rawtypes]
>> found raw type: JComboBox
>>
>>
>> Remaining issues:
>>
>> 1)
>> icedtea-web/netx/net/sourceforge/jnlp/cache/ResourceTracker.java:357: warning: [deprecation] toURL()
>> in File has been deprecated
>> return f.toURL();
>> ^
>> icedtea-web/netx/net/sourceforge/jnlp/browser/BrowserAwareProxySelector.java:205: warning:
>> [fallthrough] possible fall-through into case
>> case BROWSER_PROXY_TYPE_SYSTEM:
>> ^
>> icedtea-web/netx/net/sourceforge/jnlp/browser/BrowserAwareProxySelector.java:211: warning:
>> [fallthrough] possible fall-through into case
>> default:
>> ^
>> which is:
>>
>> @Override
>> protected List<Proxy> getFromBrowser(URI uri) {
>> List<Proxy> proxies = new ArrayList<Proxy>();
>>
>> String optionDescription = null;
>>
>> switch (browserProxyType) {
>> case BROWSER_PROXY_TYPE_PAC:
>> proxies.addAll(getFromBrowserPAC(uri));
>> break;
>> case BROWSER_PROXY_TYPE_MANUAL:
>> proxies.addAll(getFromBrowserConfiguration(uri));
>> break;
>> case BROWSER_PROXY_TYPE_NONE:
>> proxies.add(Proxy.NO_PROXY);
>> break;
>> case BROWSER_PROXY_TYPE_AUTO:
>> // firefox will do a whole lot of stuff to automagically
>> // figure out the right settings. gconf, WPAD, and ENV are used.
>> // https://bugzilla.mozilla.org/show_bug.cgi?id=66057#c32
>> // TODO this is probably not easy/quick to do. using libproxy might be
>> // the simpler workaround
>> if (optionDescription == null) {
>> optionDescription = "Automatic";
>> }
>> case BROWSER_PROXY_TYPE_SYSTEM:
>> // means use $http_proxy, $ftp_proxy etc.
>> // TODO implement env vars if possible
>> if (optionDescription == null) {
>> optionDescription = "System";
>> }
>> default:
>> if (optionDescription == null) {
>> optionDescription = "Unknown";
>> }
>>
>> OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG,R("RProxyFirefoxOptionNotImplemented",
>> browserProxyType, optionDescription));
>> proxies.add(Proxy.NO_PROXY);
>> }
>>
>> OutputController.getLogger().log("Browser selected proxies: " + proxies.toString());
>>
>> return proxies;
>> }
>>
>>
>> Well the whole code smells:
>> The first if (optionDescription == null) is always true (or not!?!?!?) so it is always
>> optionDescription = "Automatic"; Then it bubble into BROWSER_PROXY_TYPE_SYSTEM, where it can not set
>> as it is already not null. So imho both breaks are really missing. But the TODO in comments make me
>> feel uncomfortable.
>>
>>
>> 2) icedtea-web/netx/net/sourceforge/jnlp/cache/ResourceTracker.java:357: warning: [deprecation]
>> toURL() in File has been deprecated
>> icedtea-web/netx/net/sourceforge/jnlp/cache/CacheUtil.java:128: warning: [deprecation] toURL()
>> in File has been deprecated
>> icedtea-web/netx/net/sourceforge/jnlp/runtime/Boot.java:261: warning: [deprecation] toURL() in
>> File has been deprecated
>>
>>
>> There have been a lot of work around cach x file x url escaping, and as main difference between
>> file.tourl and file.touri.tourl is escapin, I rather left it.
>>
>> 3)
>> icedtea-web/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java:804: warning:
>> [deprecation] Ref in sun.misc has been deprecated
>> icedtea-web/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java:121: warning: [deprecation]
>> Ref in sun.misc has been deprecated
>>
>> What can be done?
>>
>>
>> Hope this helps,
>>
>> J.
>>
>
More information about the distro-pkg-dev
mailing list