I've only recently got commit access to libcroco, so I thought I'd start off somewhat cautiously with committing things, posting to the mailing list first, to get a better feel for tastes of the other developers etc.
I'm also trying to get simpler changes committed before more involved ones. This patch makes no change to libcroco behaviour (indeed, I've made sure that the resulting .o files are bitwise identical), it just makes things a little easier for the library user by marking certain pointer parameters as not being written through, i.e. by changing certain `MyType *' parameters to `MyType const *'. The architypal examples are adding the word `const' in the following two examples: void cr_foo_copy (CRFoo *dest, CRFoo const *src); gchar * cr_foo_to_string (CRFoo const *a_this); Adding `const' is a form of documentation, and allows calling software to use `const' more itself (whether to their own function prototypes or to objects such as `CRRgb const white = ...'), which in turn allows the compiler to catch certain types of bugs. For example, adding `const' to the cr_foo_copy allows user code to mark one of the CRFoo objects as being const, which in turn allows the compiler to flag an error if the user gets the cr_foo_copy arguments the wrong way around. Often, presence or absence of `const' hints at the ownership rules: e.g. the existing cr_string_new_from_gstring (GString *a_string) suggests that the returned CRString takes ownership of the passed a_string (i.e. that the returned CRString will copy the pointer a_string, and that it's the returned CRString's responsibility to free a_string); whereas in fact that is not the case: cr_string_new_from_gstring in fact makes a copy of the underlying string. The new prototype cr_string_new_from_gstring (GString const *a_string) makes that clearer. Having a parameter `CRFoo const *a_foo' doesn't absolutely ensure that *a_foo won't get modified (e.g. maybe there's another reference to *a_foo, or maybe someone's writing through an uninitialized pointer or the like), but it nevertheless gives some indication as to how likely it is for *a_foo to be modified, and this information helps one to decide how to direct one's time (whether to check the documentation or the implementation, or prioritizing directing debugging attention to the possibility of whether *a_foo is getting modified between here and there). Checking done on the patch: Being new, one errs towards a bit much checking of one's work before submission. As I mentioned, I've checked that autogen.sh/make results in the same set of files being produced with / without the patch, and that all regular files in the build directory are bitwise identical. (Only the libcroco-0.6.a files differed; but given that all the .o files are bitwise identical between the two versions, I suppose that the .a difference is just due to timestamps.) I checked only a single build configuration, namely CFLAGS=-O0 -g0 LDFLAGS=-s. I tried checking that the diff consists essentially of adding `const' to things (and stripping trailing whitespace) by making a copy of both the before and after versions and doing perl -pi -e 's/const //g;s/ $//' src/cr-*.[ch] and then doing a diff between the two resulting trees, i.e. seeing what changes the patch makes other than adding/removing const or trailing space. These are: - Removing some non-const casts that become unnecessary (in cr-fonts.c); - Trivial whitespace fix to cr_rgb_copy second parameter; - Splitting a declaration of two variables into two statements in two places in src/cr-utils.c (due to the two variables now having different cv qualifiers). In the interest of whitespace consistency, I've checked that none of the `+' lines in the diff contain trailing whitespace, and none of the `+' lines contain tabs except where the surrounding code is also using tabs for indentation. The patch is 60KiB long, so I won't attach it here; it's available at http://bowman.infotech.monash.edu.au/~pmoulder/libcroco-const-diffs . I'll commit it in a few days (no hurry). pjrm. _______________________________________________ Libcroco-list mailing list Libcroco-list@gnome.org http://mail.gnome.org/mailman/listinfo/libcroco-list