.gnucash and GNC_DOT_DIR

John Ralls jralls at ceridwen.us
Fri Feb 5 16:43:42 EST 2010


On Feb 4, 2010, at 5:53 PM, Andreas Köhler wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi John,
> 
> I like code to be improved, but a few things come to my mind. You can
> probably fix them rather easily.
> 
> - - Watch out for memory leaks. E.g. you must free the result of
> gnc_path_get_pkgdatadir() after usage, otherwise you leak it. The
> function documentation typically explains the memory management of
> parameters and return values.
> - - Try to use glib functionality to avoid mistakes and improve
> portability. E.g. if you do not explicitly pass NULL to getcwd(2) then
> you are expected to provide a char[] buffer for the function to fill in.
> g_get_current_dir() is the solution and devhelp your friend :)
> - - BEGIN and LEAVE should wrap all possible paths through a function (if
> needed at all). I.e., start with BEGIN and add LEAVE before every return
> or the end of the function.
> - - I am picky wrt trailing whitespaces, no tab characters unless already
> in the surrounding code, printfs (like g_printerr), unneeded code newly
> commented out
> 
> I am more interested whether the removed functionality really is not
> needed anymore or fully replaced. Is .gnucash created in time, do we
> need to support file: or xml: schemas anywhere?
> 
> Personally, I could not guess about the patch on Windows, someone (not
> me ;-)) would need to give it a try.
> 

Thanks for the reminder about freeing g_strings, and I agree that g_get_current_dir() is a better choice than getcwd(). 

It's ENTER and LEAVE, I think, and your point is well taken (and fixed).

That diagnostic g_printerr() got left in by mistake. Thanks for pointing it out. I'm not terribly concerned about trailing whitespace in C, it's not significant -- but I ran delete-trailing-whitespace on the buffer anyway. It will add some bogus lines to the diff, but is otherwise harmless.

~/.gnucash (or whatever $GNC_DOT_DIR is, which is the point of the changes) is handled by gnc_build_dotgnucash_dir(). The deleted MakeHomeDir() duplicated gnc_build_dotgnucash_dir()'s functionality, but did so with hard-coded paths.

The URI scheme detection code (for file: and xml:) was already handled in the companion function in the file, xaccResolveURL(). xaccResolveFileName() is used twice directly, and in both cases if a URI is provided instead of a filename, other parts of the calling function will fail. This patch still passes src/engine/test/test-resolve-file-path, which looks like the applicable unit test.

The new patch is attached below. I'm still looking for some feedback from an M$Win build before I commit.

Regards,
John Ralls


-------------- next part --------------
A non-text attachment was scrubbed...
Name: dot_gnucash.patch
Type: application/octet-stream
Size: 11392 bytes
Desc: not available
URL: <http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20100205/59e56dbc/attachment.obj>


More information about the gnucash-devel mailing list