policy/style clarification

Derek Atkins warlord at MIT.EDU
Fri May 30 10:54:24 CDT 2003


Hi,

"Levi Bard" <levi at bard.sytes.net> writes:

> I have a quick question regarding policy/style of patches.  Is it
> acceptable to submit patches to bring code in line with the policies
> described in HACKING?  For example, replacing a large number of `if (
> !blah || !foo ) return;` with `g_return_if_fail(blah && foo);` , or
> `if(blah) free(blah);` with `g_free(blah);` ?  Or is this pedantic and
> unhelpful?

Well, those changes are not always correct.  For example,
g_return_if_fail() is an assertion check.  For public APIs you should
not assert that the "user" of the API gave reasonable input.  You
should fail gracefully instead of failing with an assertion.  For
internal functions you can use the assertions.

Similarly, not all free()'s should be g_free().  It depends how the
string was malloc()'ed.  If you find places where a string was
malloc'ed using g_malloc() (or g_new(), or any other g_*() function)
and then freed by free() then yes, that should be corrected.  But
gratuitously changing free() to g_free() is not correct.

> Levi

-derek

-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord at MIT.EDU                        PGP key available


More information about the gnucash-patches mailing list