[libcroco-list] adding `const' in prototypes



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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]