Uemis patches

Dirk Hohndel dirk at hohndel.org
Mon Sep 7 07:20:21 PDT 2015


On Mon, Sep 07, 2015 at 05:14:42PM +0300, Miika Turkia wrote:
> 
> +        /* quickhack and workaround to capture the original dive_no
> +         * i am doing this so I dont have to change the original design
> +         * but when parsing a dive we never parse the dive number because
> +         * at the time it's being read the *dive varible is not set because
> +         * the dive_no tag comes before the object_id in the uemis ans file
> +         */
> +        char *dive_no_buf = strdup(inbuf);
> +        char *dive_no_ptr = strstr(dive_no_buf, "dive_no{int{") + 12;
> 
> You should ensure the strstr returns a valid pointer and not null. If
> null is returned, the input file is not valid and you should take that
> into account. (I have implemented way too many bugs myself when
> importing data with no proper validation - the input just isn't always
> according to the spec.)

Very good point. I don't think I've seen Uemis files with partial tag
sets, but that doesn't mean they don't exist.
> 
> +        char *dive_no_end = strstr(dive_no_ptr, "{");
> 
> Are you really looking for { and not closing char? And you should

No, it really is all delimited by left curly braces. Uemis' data format
is... rather excentric.

> again make sure the return value is not null. I believe strchr would
> be faster when you are searching for one character only.
> 
> +        *dive_no_end = 0;
> 
> You might segm fault if you got null in the previous search.
> 
> +        strcpy(dive_no, dive_no_ptr);
> 
> I suggest you use strncpy instead of strcpy to avoid buffer overflows.
> (Note that strncpy does not null terminate if the input length is n or
> more.)

ALl great feedback, miika. Thanks for adding this - I'm still focused too
much on the style and not enough on the substance when reviewing Guido's
code.

/D


More information about the subsurface mailing list