[sip-comm-dev] [Patch] Fix lot of warnings in the code


#1

Hi SIP Communicator developers,

I have found some time to look at the code and fix a lot of warnings.
Attached to this e-mail a patch against revision 5886 of trunk that drop
warning number to 327 (before it was over 1000). The fixes are primary for
generics stuff (Hashtable o => Hashtable<String, String> o...), I will try
to fix other warnings next week if I have time.

So please review this patch and give feedback. I hope it will be applied.

Best regards,

sip-communicator-patch-warnings2.diff (301 KB)

···

--
Sebastien Vincent
Manager of TurnServer project (http://www.turnserver.org/)


#2

I'm working on the integration of this patch into trunk now. Just to
let other devs know because it's going to take some time since there
are nearly 200 modified files and there's at least one endless
recursion.

···

On Mon, Aug 31, 2009 at 10:17 AM, Sebastien Vincent<sebastien.vincent@cppextrem.com> wrote:

Hi SIP Communicator developers,

I have found some time to look at the code and fix a lot of warnings.
Attached to this e-mail a patch against revision 5886 of trunk that drop
warning number to 327 (before it was over 1000). The fixes are primary for
generics stuff (Hashtable o => Hashtable<String, String> o...), I will try
to fix other warnings next week if I have time.

So please review this patch and give feedback. I hope it will be applied.

Best regards,
--
Sebastien Vincent
Manager of TurnServer project (http://www.turnserver.org/)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#3

Hi Sebastien,

OMG! Thank you very much for the excellent work!

I just finished integrating your patch into trunk and acknowledged
your contribution on our Team and Contributors page at
http://www.sip-communicator.org/index.php/Development/TeamAndContributors.

I hope you will not mind that I applied a few modifications to your
patch and I just want to share a summary of them here for the sake of
clarity, I definitely don't want to take away any of the excellency of
your work.

- I replaced your AccountList modifications with mine because the
elements() method you had added, recursively called itself instead of
calling the super implementation and thus caused an infinite
recursion.

- I removed a List type specialization in a class related to
server-side contact details (I think) because you had written new
List<String>(result) while result contained elements of type different
than String and it would cause a ClassCastException at runtime.

- I removed a couple of type specializations related to presence in
the tests because the casts wouldn't go through at runtime (though it
seems natural for them to).

- I fixes many other warnings in order to increase my chances of
discovering problems such as the above two which the compiler warns
about but get lost in the flood of other warnings.

- I fixed at least four cases of incorrect implementations of
multi-user chat related methods for retrieving the joined ChatRooms
that would through ClassCastExceptions at runtime. These were not
caused by you but there were modifications in some of them I think so
I noticed the problems.

- As we try to follow a coding convension (published at
http://www.sip-communicator.org/index.php/Documentation/CodeConvention),
I fixed the line wrapping in multiple places.

I guess I'm forgetting something but the number of modifications made
by the patch was huge...

Thank you again! Looking forwards to reviewing more of your patches :slight_smile:

Regards,
Lubomir

···

On Mon, Aug 31, 2009 at 10:17 AM, Sebastien Vincent<sebastien.vincent@cppextrem.com> wrote:

Hi SIP Communicator developers,

I have found some time to look at the code and fix a lot of warnings.
Attached to this e-mail a patch against revision 5886 of trunk that drop
warning number to 327 (before it was over 1000). The fixes are primary for
generics stuff (Hashtable o => Hashtable<String, String> o...), I will try
to fix other warnings next week if I have time.

So please review this patch and give feedback. I hope it will be applied.

Best regards,
--
Sebastien Vincent
Manager of TurnServer project (http://www.turnserver.org/)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#4

Hi,

I just find that patch against
src/net/java/sip/communicator/impl/gui/main/account/AccountList.java file
causes bug. So just avoid diff of this file.

Regards,

···

--
Sebastien Vincent

On Mon, 31 Aug 2009 11:33:16 +0300, Lubomir Marinov <lubomir.marinov@gmail.com> wrote:

I'm working on the integration of this patch into trunk now. Just to
let other devs know because it's going to take some time since there
are nearly 200 modified files and there's at least one endless
recursion.

On Mon, Aug 31, 2009 at 10:17 AM, Sebastien > Vincent<sebastien.vincent@cppextrem.com> wrote:

Hi SIP Communicator developers,

I have found some time to look at the code and fix a lot of warnings.
Attached to this e-mail a patch against revision 5886 of trunk that drop
warning number to 327 (before it was over 1000). The fixes are primary
for
generics stuff (Hashtable o => Hashtable<String, String> o...), I will
try
to fix other warnings next week if I have time.

So please review this patch and give feedback. I hope it will be

applied.

Best regards,
--
Sebastien Vincent
Manager of TurnServer project (http://www.turnserver.org/)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#5

Hi,

Here another patch I made against revision 5892. Now we have 67 warnings
:D.

It remains some warnings. Here is a summary:

- Fallthrough warnings:
=>
src/net/java/sip/communicator/plugin/whiteboard/gui/WhiteboardFrame.java
(line 559 and 562), could be normal but need to add a comment (like in
src/net/java/sip/communicator/impl/gui/main/contactlist/ContactList.java
line 1407);
=>
src/net/java/sip/communicator/impl/media/transform/srtp/SRTPCryptoContext.java
(line 243), again if maintainer could add a comment if it is normal.

- Warnings in src/net/java/sip/communicator/impl/media/
=> Related to JMF library that do not support generics. We could get rid
of them by @SuppressWarning but it could hide future warnings if we made
modifications on this files later.

- Java API parent class that do not return generics:
=>
src/net/java/sip/communicator/impl/gui/customcontrols/ExtendedTableModel.java
(line 30)
=>
src/net/java/sip/communicator/impl/gui/main/chat/conference/ChatInviteDialog.java
(line 269)

sip-communicator-patch-warnings-bis.diff (76.5 KB)

···

-
src/net/java/sip/communicator/impl/gui/main/call/TransferCallButton.java:176:
=> Could be fixed in we change OperationSetBasicTelephony::getActiveCalls
to return Iterator<Call> and change all subclasses.
SIP and Jabber ones has return type Iterator<Call[Jabber|Sip]Impl> for this
method. So if it is OK to change it to Iterator<Call> and cast to
Call...Impl if needed, we will have less warnings. WDYT ?

-
src/net/java/sip/communicator/impl/protocol/facebook/FacebookBuddyList.java
=> Related to JSONObject that do not return generics

- Warnings caused by lib that do not return generics:
=> src/net/java/sip/communicator/impl/protocol/rss/RssFeedReader.java:130:
=>
src/net/java/sip/communicator/impl/protocol/sip/SipStackSharing.java:348:
=> Warnings in src/impl/yahoo

- Use of Sun API
=>
/home/vincent/svn_work/sip-communicator/src/net/java/sip/communicator/util/FileUtils.java
(line 63 and 64).

Best regards,
--
Sebastien Vincent

On Mon, 31 Aug 2009 18:52:45 +0300, Lubomir Marinov <lubomir.marinov@gmail.com> wrote:

Hi Sebastien,

OMG! Thank you very much for the excellent work!

I just finished integrating your patch into trunk and acknowledged
your contribution on our Team and Contributors page at

http://www.sip-communicator.org/index.php/Development/TeamAndContributors.

I hope you will not mind that I applied a few modifications to your
patch and I just want to share a summary of them here for the sake of
clarity, I definitely don't want to take away any of the excellency of
your work.

- I replaced your AccountList modifications with mine because the
elements() method you had added, recursively called itself instead of
calling the super implementation and thus caused an infinite
recursion.

- I removed a List type specialization in a class related to
server-side contact details (I think) because you had written new
List<String>(result) while result contained elements of type different
than String and it would cause a ClassCastException at runtime.

- I removed a couple of type specializations related to presence in
the tests because the casts wouldn't go through at runtime (though it
seems natural for them to).

- I fixes many other warnings in order to increase my chances of
discovering problems such as the above two which the compiler warns
about but get lost in the flood of other warnings.

- I fixed at least four cases of incorrect implementations of
multi-user chat related methods for retrieving the joined ChatRooms
that would through ClassCastExceptions at runtime. These were not
caused by you but there were modifications in some of them I think so
I noticed the problems.

- As we try to follow a coding convension (published at
http://www.sip-communicator.org/index.php/Documentation/CodeConvention),
I fixed the line wrapping in multiple places.

I guess I'm forgetting something but the number of modifications made
by the patch was huge...

Thank you again! Looking forwards to reviewing more of your patches :slight_smile:

Regards,
Lubomir

On Mon, Aug 31, 2009 at 10:17 AM, Sebastien > Vincent<sebastien.vincent@cppextrem.com> wrote:

Hi SIP Communicator developers,

I have found some time to look at the code and fix a lot of warnings.
Attached to this e-mail a patch against revision 5886 of trunk that drop
warning number to 327 (before it was over 1000). The fixes are primary
for
generics stuff (Hashtable o => Hashtable<String, String> o...), I will
try
to fix other warnings next week if I have time.

So please review this patch and give feedback. I hope it will be

applied.

Best regards,
--
Sebastien Vincent
Manager of TurnServer project (http://www.turnserver.org/)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#6

Hi Sebastien,

Thank you very much for the continued contribution! It has been committed into trunk as r5893 and acknowledged on our Team and Contributors page.

Regards,
Lubomir

Sebastien Vincent написа:

···

Hi,

Here another patch I made against revision 5892. Now we have 67 warnings
:D.

It remains some warnings. Here is a summary:

- Fallthrough warnings:
=>
src/net/java/sip/communicator/plugin/whiteboard/gui/WhiteboardFrame.java
(line 559 and 562), could be normal but need to add a comment (like in
src/net/java/sip/communicator/impl/gui/main/contactlist/ContactList.java
line 1407);
=>
src/net/java/sip/communicator/impl/media/transform/srtp/SRTPCryptoContext.java
(line 243), again if maintainer could add a comment if it is normal.

- Warnings in src/net/java/sip/communicator/impl/media/
=> Related to JMF library that do not support generics. We could get rid
of them by @SuppressWarning but it could hide future warnings if we made
modifications on this files later.

- Java API parent class that do not return generics:
=>
src/net/java/sip/communicator/impl/gui/customcontrols/ExtendedTableModel.java
(line 30)
=>
src/net/java/sip/communicator/impl/gui/main/chat/conference/ChatInviteDialog.java
(line 269)

-
src/net/java/sip/communicator/impl/gui/main/call/TransferCallButton.java:176:
=> Could be fixed in we change OperationSetBasicTelephony::getActiveCalls
to return Iterator<Call> and change all subclasses.
SIP and Jabber ones has return type Iterator<Call[Jabber|Sip]Impl> for this
method. So if it is OK to change it to Iterator<Call> and cast to
Call...Impl if needed, we will have less warnings. WDYT ?

-
src/net/java/sip/communicator/impl/protocol/facebook/FacebookBuddyList.java
=> Related to JSONObject that do not return generics

- Warnings caused by lib that do not return generics:
=> src/net/java/sip/communicator/impl/protocol/rss/RssFeedReader.java:130:
=>
src/net/java/sip/communicator/impl/protocol/sip/SipStackSharing.java:348:
=> Warnings in src/impl/yahoo

- Use of Sun API
=>
/home/vincent/svn_work/sip-communicator/src/net/java/sip/communicator/util/FileUtils.java
(line 63 and 64).

Best regards,
--
Sebastien Vincent

On Mon, 31 Aug 2009 18:52:45 +0300, Lubomir Marinov > <lubomir.marinov@gmail.com> wrote:

Hi Sebastien,

OMG! Thank you very much for the excellent work!

I just finished integrating your patch into trunk and acknowledged
your contribution on our Team and Contributors page at

http://www.sip-communicator.org/index.php/Development/TeamAndContributors.

I hope you will not mind that I applied a few modifications to your
patch and I just want to share a summary of them here for the sake of
clarity, I definitely don't want to take away any of the excellency of
your work.

- I replaced your AccountList modifications with mine because the
elements() method you had added, recursively called itself instead of
calling the super implementation and thus caused an infinite
recursion.

- I removed a List type specialization in a class related to
server-side contact details (I think) because you had written new
List<String>(result) while result contained elements of type different
than String and it would cause a ClassCastException at runtime.

- I removed a couple of type specializations related to presence in
the tests because the casts wouldn't go through at runtime (though it
seems natural for them to).

- I fixes many other warnings in order to increase my chances of
discovering problems such as the above two which the compiler warns
about but get lost in the flood of other warnings.

- I fixed at least four cases of incorrect implementations of
multi-user chat related methods for retrieving the joined ChatRooms
that would through ClassCastExceptions at runtime. These were not
caused by you but there were modifications in some of them I think so
I noticed the problems.

- As we try to follow a coding convension (published at
http://www.sip-communicator.org/index.php/Documentation/CodeConvention),
I fixed the line wrapping in multiple places.

I guess I'm forgetting something but the number of modifications made
by the patch was huge...

Thank you again! Looking forwards to reviewing more of your patches :slight_smile:

Regards,
Lubomir

On Mon, Aug 31, 2009 at 10:17 AM, Sebastien >> Vincent<sebastien.vincent@cppextrem.com> wrote:

Hi SIP Communicator developers,

I have found some time to look at the code and fix a lot of warnings.
Attached to this e-mail a patch against revision 5886 of trunk that drop
warning number to 327 (before it was over 1000). The fixes are primary
for
generics stuff (Hashtable o => Hashtable<String, String> o...), I will
try
to fix other warnings next week if I have time.

So please review this patch and give feedback. I hope it will be

applied.

Best regards,
--
Sebastien Vincent
Manager of TurnServer project (http://www.turnserver.org/)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net