[CCR PATCH] set dc type within the dc structure

[Willem Ferguson]

On 10/06/2014 17:46, Dirk Hohndel wrote:
> 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>
>> ---
>> @@ -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.
Your point is taken seriously.
>
>> 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
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?

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

Kind regards,
willem