Bug 2617 - code cleanup
Summary: code cleanup
Status: RESOLVED FIXED
Alias: None
Product: Claws Mail
Classification: Unclassified
Component: Other (show other bugs)
Version: 3.8.1
Hardware: All All
: P3 minor
Assignee: users
URL:
Depends on:
Blocks:
 
Reported: 2012-02-28 14:28 CET by Christian Hesse
Modified: 2012-11-24 10:12 CET (History)
0 users

See Also:


Attachments
code cleanup (58.92 KB, patch)
2012-02-28 14:28 CET, Christian Hesse
no flags Details | Diff
code cleanup (57.50 KB, patch)
2012-02-28 20:46 CET, Christian Hesse
no flags Details | Diff
Code cleanup (58.11 KB, patch)
2012-03-20 15:23 CET, Christian Hesse
no flags Details | Diff
Code cleanup (12.07 KB, patch)
2012-11-23 13:21 CET, Christian Hesse
no flags Details | Diff
Code cleanup (3.58 KB, patch)
2012-11-23 17:11 CET, Christian Hesse
no flags Details | Diff
Code cleanup (15.35 KB, patch)
2012-11-23 18:21 CET, Christian Hesse
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Hesse 2012-02-28 14:28:08 CET
Created attachment 1087 [details]
code cleanup

This cleans up the code a bit. Compiler warnings, unused variables, ... A lot of cosmetic stuff that has no effect for the end user.

This works for me, but please take a look at it before applying.

This are still a lot for warnings left, will looks at that after this has been applied.
Comment 1 wwp 2012-02-28 15:40:29 CET
This one doesn't make sense to me, as color[] is used whatever the GTK+ version is, a contrario to cmap, success[] and i which are used when GTK+ version differs from 2.24.0. Won't compile if GTK+ version is 2.24.0.


--- src/compose.c	2011-12-30 06:00:40.000000000 +0100
+++ src/compose.c	2012-02-28 11:35:23.126835114 +0100
@@ -801,8 +801,8 @@ static void compose_create_tags(GtkTextV
 	GdkColormap *cmap;
 	gboolean success[8];
 	int i;
-#endif
 	GdkColor color[8];
+#endif
 
 	buffer = gtk_text_view_get_buffer(text);
Comment 2 wwp 2012-02-28 15:42:34 CET
What's your GCC and GTK+ versions? It may explain some hunks in your patch!
Comment 3 wwp 2012-02-28 15:44:36 CET
Also, I think that removing src/gtk/headers.h will break translations.
Comment 4 Colin Leroy 2012-02-28 15:48:16 CET
Re comment #3: yes, it will.
Comment 5 Christian Hesse 2012-02-28 15:50:39 CET
gcc 4.6.2-7
gtk2 2.24.10-3

This is an Arch Linux system.

Without the change to src/compose.c I got:
warning: variable 
Comment 6 wwp 2012-02-28 15:52:14 CET
Nice catch for #define ICONS 21! :-)
Comment 7 wwp 2012-02-28 15:56:02 CET
Oh, yes you're right for color[] being unused in such case, sorry. Since it's not static, it's really set for nothing or I missed something.
Comment 8 Colin Leroy 2012-02-28 16:05:46 CET
-	c = read(source, &buf, 1);
+	read(source, &buf, 1);

These are there to silence warnings about non-used return values. Of course, now there are unused variables warnings. The best would be to actually check everything went fine.
Comment 9 Christian Hesse 2012-02-28 16:37:05 CET
My gcc is happy without checking for return values. :D

Ok, so all of these should look like this?

if (read(source, &buf, 1) < 0)
  g_warning("count not read.\n");
Comment 10 Colin Leroy 2012-02-28 16:41:07 CET
Yep. Actually, use debug_print(), because these really shouldn't fail - otherwise they would be checked :)
Comment 11 Christian Hesse 2012-02-28 20:46:47 CET
Created attachment 1088 [details]
code cleanup

Removed everything related to headers.h (though gcc complains about unused 'HEADERS' now) and added error handling. Hope I got everything right.
Comment 12 Christian Hesse 2012-03-20 15:23:28 CET
Created attachment 1092 [details]
Code cleanup

Rebases on current cvs...

Last activity was two and a half week ago... Still any objections or will this be merged?
Comment 13 Abhay S. Kushwaha 2012-11-23 11:18:23 CET
Christian, why don't you suggest the patch again, this time against the current CVS and let's see if we can get the developers interested? :)
Comment 14 Christian Hesse 2012-11-23 11:48:29 CET
Oh, I see a lot of "Reversed (or previously applied) patch detected!" messages... When did that happen?
I will try to find some time to rebase the patch.
Comment 15 Christian Hesse 2012-11-23 13:21:53 CET
Created attachment 1193 [details]
Code cleanup

Ok, most of this as been applies or been dealt with. I have created a new patch against 3.9.0cvs18 with what remains.
Comment 16 Colin Leroy 2012-11-23 16:01:56 CET
Hi Christian,

Thanks for taking the time to update the patch. Just a few things:

for src/gtk/authors.h, the list is alphebetized (by last name)

I don't understand the need for the GtkAdjustment/GtkObject replacements either? There are no warnings as the code is. (There may have been at the time of first patch submittal though)
Comment 17 Christian Hesse 2012-11-23 17:11:58 CET
Created attachment 1194 [details]
Code cleanup

I am pretty sure I had some warnings about GtkAdjustment being deprecated... But to be honest: I can not find any documentation about that.

Ok, updated again:

* fixed alphabetical order in src/gtk/authors.h
* removed everything about GtkAdjustment
* added a real fix for gnutls
Comment 18 Colin Leroy 2012-11-23 17:36:31 CET
Thanks again!

I have build errors with the patch applied though. Mostly the removal of r and error cause problem as it seems we're now using these variables :)

I've applied the AUTHORS part anyway as it seems we forgot to add you in the beginning!
Comment 19 Christian Hesse 2012-11-23 18:21:12 CET
Created attachment 1195 [details]
Code cleanup

Ok, last patch had some problems... Back in march some variables were unused but are no longer in latest version...

Additionally my fix for gnutls was wrong. We should keep it this way if we want to be compatible with gnutls <= 2.10.
But I changed type to not use the deprecated ones from gnutls 1.x.

So here is an updated version.

There are some more glib and gtk warnings, but I will look at these if we got this right.
Comment 20 users 2012-11-23 20:30:34 CET
Changes related to this bug have been committed.
Please check latest CVS and update the bug accordingly.
You can also get the patch from:
http://www.claws-mail.org/tracker/

2012-11-23 [colin]	3.9.0cvs20

	* src/jpilot.c
	* src/main.c
	* src/mainwindow.c
	* src/matcher.c
	* src/prefs_common.c
	* src/prefs_common.h
	* src/procmsg.c
	* src/common/socket.c
	* src/common/socket.h
	* src/common/ssl.c
	* src/common/ssl_certificate.c
	* src/common/ssl_certificate.h
	* src/etpan/imap-thread.c
	* src/etpan/nntp-thread.c
		Cleanup some warnings and deprecated types. Patch by
		Christian Hesse, fixing bug #2617. Thanks!