Uemis patches

[Guido Lerch]

2015-09-07 16:20 GMT+02:00 Dirk Hohndel <dirk at hohndel.org>:

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

True, but I am actually doing the validation at another point, so if the
code is being executed
I have assured that my pointers do not return NULL.


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

Dirk is correct, the Uemis has a VERY weird data format


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

Agree, create feedback, I'll have one more patch with the whitespace stuff
unfortunately or can
I skip a commit when creating a new patch or rename the patch ?

Again, I am new to all this.

Thanks for helping.


>
> /D
>



-- 
Best regards,
Guido
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20150907/5b28554e/attachment.html>