[CCR PATCH] set dc type within the dc structure

[Dirk Hohndel]

On Tue, Jun 10, 2014 at 08:44:34PM +0200, Willem Ferguson wrote:
> >>+++ b/parse-xml.c
> >>@@ -531,7 +531,7 @@ static void get_rating(char *buffer, void *_i)
> >>  static void double_to_permil(char *buffer, void *_i)
> >>  {
> >>-	int *i = _i;
> >>+	uint16_t *i = _i;
> >And this could cause the same crash / corruption that Linux just fixed.
> >Please don't mix pointers with targets of different sizes.
> >
> >>+/* Parse the dc_type string from xml and set the dc type.. */
> >>+static void dctype_string(char *buffer, void *_d)
> >>+{
> >>+	int size;
> >>+	char *res;
> >>+        enum dive_comp_type *d = _d;                    // define a dive_computer_type
> >Again, really bad idea. Don't use a void * as function argument, please
> If one looks through parse-xml.c, then there is a host of void functions
> that also use
> void as an argument. I modelled the function here on those functions. The
> way that
> I would normally do this is for the function to return a value rather than
> just to
> assign a value to a void passed as a parameter to a void function. But this
> would
> mean a change in coding style for a substantial number of functions in that
> file.
> Do you perceive this as the way forward?

Are you on the latest master? Almost all of these function calls have been
changed since your last set of patches caused crashes...

The latest master should only use void* when we pass much bigger
structures around. Everything else is typed.

> I will work on those two functions (double_to_permil and dctype_string)
> but to do the rest of these functions should probably be a separate
> exercise. Does this sound ok?

Look at what we have now and model based on that, please.

> >>+	size = strlen(buffer);
> >>+	while (size && isspace(buffer[size - 1]))
> >>+		size--;
> >>+	if (!size)
> >>+		return;
> >Maybe we should have this whole "remove whitespace around a value" in a
> >helper function...
> >
> There is at least one other closely-situated function that would benefit
> from such
> a helper. Shall I do this?

Yes, please

/D