Code cleanup
I just had a look at the code of some of the plug-ins and I noticed that there is often lot's of room for improvement: 1) some constructions are plain clumsy, for example somewhere I saw: for (k = 0; k bytes; k++) destline++; instead of simply: destline += bytes; 2) a lot of functionality has been added since the last update of some of the plug-ins. Result: duplicate functionality 3) sometimes it's just lack of C knowledge: if (p) free(p); can be simply replaced by just: free(p); No need for the test, unless performance is really critical and we expect to free a lot of NULL pointers. 4) some plugins have obvious memory leaks. So my question: is it worth the effort to carefully go through all the code (not only plug-ins) and clean things up? Advantages: 1) source code becomes smaller. Not a big deal with Gb harddisks, but nice for future maintenance. 2) object code becomes smaller. My initial estimate that for example most plug-ins can be easily reduced with 10 a 20 % without much effort. 3) during the process we might find further bugs. Disadvantages: 1) this will cost time/effort (I am willing to make my contribution). 2) we might break things that work 3) we don't spend time on other fun things liking adding functionality. Any thoughts/comments?
Re: Code cleanup
Federico Mena Quintero wrote:
snip
3) sometimes it's just lack of C knowledge:
if (p)
free(p);
can be simply replaced by just:
free(p);
If p is a null pointer then "free (p)" may (and should!) crash. You
are incorrect here.
Hm. According to most (all?) books I have and the manpages it is
perfectly legal to do a free(NULL). Anyhow, it's better to use g_free
instead to be on the save side.
snip
2) we might break things that work
This can be fixed by, you know, testing things after you change them :-)
Agree.
3) we don't spend time on other fun things liking adding functionality.
The GIMP does not need more functionality at this point; it needs to
be checked for correctness.
I fully agree. I am just trying to make the point that by carefully
reviewing existing code we might find more bugs and in the meanwhile can
clean-up the code a bit. By cleaning up the code I don't mean extensive
re-writes or adding functionality, but just fixing obvious stuff.
(Although I realize that even fixing obvious stuff can result in bugs
;-) )
As an example of what I mean: somewhere in the xpm plugin (xpm.c) you
will find the following code:
/* get some basic stats about the image */
switch (gimp_drawable_type (drawable_ID))
{
case RGBA_IMAGE:
case INDEXEDA_IMAGE:
case GRAYA_IMAGE:
alpha = TRUE;
break;
case RGB_IMAGE:
case INDEXED_IMAGE:
case GRAY_IMAGE:
alpha = FALSE;
break;
default:
return FALSE;
}
switch (gimp_drawable_type (drawable_ID))
{
case GRAYA_IMAGE:
case GRAY_IMAGE:
color = FALSE;
break;
case RGBA_IMAGE:
case RGB_IMAGE:
case INDEXED_IMAGE:
case INDEXEDA_IMAGE:
color = TRUE;
break;
default:
return FALSE;
}
switch (gimp_drawable_type (drawable_ID))
{
case GRAYA_IMAGE:
case GRAY_IMAGE:
case RGBA_IMAGE:
case RGB_IMAGE:
indexed = FALSE;
break;
case INDEXED_IMAGE:
case INDEXEDA_IMAGE:
indexed = TRUE;
break;
default:
return FALSE;
}
This could be replaced by (sure hope I am not making mistakes now):
alpha = gimp_drawable_has_alpha (drawable_ID);
color = gimp_drawable_is_rgb (drawable_ID);
indexed = gimp_drawable_is_indexed (drawable_ID);
5 minutes work, no change in functionality, sourcecode a bit shorter
(and much more readable) and a few hundred bytes less objectcode.
Maurits
Announcement: release 0.2 of the Splitter plug-in
Hi, Release 0.2 of my Splitter plug-in is available. Some highlights: * Added functions acos, asin, atan, atan2, cosh, sinh, tanh and ldexp. * Option to set random seed (via number or current time). * Option to keep layer 1, layer 2 or both layers. * Pictures with alpha channel now handled correctly. * Plug-in can now handle non-interactive calls. * Improved handling of scientific notation for numbers (e.g. 1e-2) More information and examples can be found on http://home-2.consunet.nl/~cb007736/splitter.html Maurits
