[CCR PATCH] set dc type within the dc structure

[Dirk Hohndel]

On Tue, Jun 10, 2014 at 03:54:33PM +0200, Willem Ferguson wrote:
> This patch lays the foundation for differentiating between open-circuit(OC)
> dives and rebreather dives (CCR). The following were done:
> 1) In dive.h add an enum type dive_computer_type
> 2) In dive.h add two more fields to the dc structure:
> 	a) dctype  (an enum field indicating dc type)
> 	b) no_o2sensors (indicating number of o2 sensors for this dc)
> 3) In parse-xml.c add a function dctype_string. This parses the dc_type
>    string from xml and assigns an enum value which will later be returned
>    to the function that parses the dc variables.
> 4) In dive.c in function fixup_dive_dc, the dc->dctype variable is set
>    to OC (open-circuit) if this value has not been initialised from the
>    xm log file.
> 
> Signed-off-by: Willem Ferguson <willemferguson at zoology.up.ac.za>
> ---
>  dive.c      |    3 +++
>  dive.h      |    5 +++++
>  parse-xml.c |   24 +++++++++++++++++++++++-
>  3 files changed, 31 insertions(+), 1 deletion(-)

I see no change to save-xml (or to load-git / save-git), so I'm guessing
this is a partial implementation?

> diff --git a/dive.c b/dive.c
> index 1f1a597..b308027 100644
> --- a/dive.c
> +++ b/dive.c
> @@ -851,6 +851,9 @@ static void fixup_dive_dc(struct dive *dive, struct divecomputer *dc)
>  	fixup_dc_duration(dc);
>  
>  	update_min_max_temperatures(dive, dc->watertemp);
> +
> +        if (!dc->dctype) dc->dctype = OC;       // If dc type is undefined, then make it open circuit (OC)
> +                                                // This ensures dc->dctype always has a value.

This is pretty redundant since OC == 0 -- and that's the right way to do
this. Have the default (null) value be what you want to be the default. OC
in this case.
So just drop this line. We zero out the dc when it's allocated.

> diff --git a/dive.h b/dive.h
> index b7a53d5..256336f 100644
> --- a/dive.h
> +++ b/dive.h
> @@ -41,6 +41,9 @@ extern "C" {
>  #include <stdbool.h>
>  #endif
>  
> +enum dive_comp_type {OC, CCR};        // Flags (Open-circuit and Closed-circuit-rebreather) for setting dive computer type

Since the standard states that enum values (unless explicitly set) start
with 0, we have exactly what we want. OC == 0 and so the code above is
redundant.

> @@ -216,6 +219,8 @@ struct divecomputer {
>  	depth_t maxdepth, meandepth;
>  	temperature_t airtemp, watertemp;
>  	pressure_t surface_pressure;
> +        enum dive_comp_type dctype;     // Type of dive computer: 'CCR' = rebreather
> +        int no_o2sensors;               // rebreathers: number of O2 sensors used

I'm guessing we have some white space issue here. Actually, looking
through the patch all indenting is done with spaces.

Not OK.

> diff --git a/parse-xml.c b/parse-xml.c
> index 1b5d88b..e89f90f 100644
> --- a/parse-xml.c
> +++ 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

> +	while (isspace(*buffer))                        // Collect the string from xml buffer
> +		buffer++;

The comment certainly doesn't appear to match the code (which is skipping
leading whitespace.

> +	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...

> +	res = malloc(size + 1);                         // Store the string..
> +	memcpy(res, buffer, size);
> +	res[size] = 0;                                  // ..and terminate it properly

It's such a pain that MacOS doesn't have strndup().

> +        if (strcmp(res,"CCR") == 0)                     // Based on this string..
> +                *d = CCR;                               // .. set the dc type
> +        else                                            // If the string is not 'CCR'..
> +                *d = OC;                                // then set it to OC (open-circuit)
> +}

Overall: Whitespace. It really isn't so hard. Please set up your editor
and it will do 2/3 of the work for you.

/D