On Wed, May 11, 2022 at 8:02 AM Zane van Iperen <z...@zanevaniperen.com> wrote:
>
>
>
> On 10/5/22 23:18, Andreas Rheinhardt wrote:
>
> >> +int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID
> >> uu)
> >> +{
> >> + int i;
> >> + const char *cp;
> >> + char buf[3];
> >> +
> >> + if ((in_end - in_start) != 36)
> >> + return -1;
> >> +
> >> + for (i = 0, cp = in_start; i < 36; i++, cp++) {
> >> + if (i == 8 || i == 13 || i == 18 || i == 23) {
> >> + if (*cp == '-')
> >> + continue;
> >> + return AVERROR(EINVAL);
> >> + }
> >> +
> >> + if (!av_isxdigit(*cp))
> >> + return AVERROR(EINVAL);
> >> + }
> >> +
> >> + buf[2] = '\0';
> >> + for (i = 0, cp = in_start; i < 16; i++) {
> >> + if (i == 4 || i == 6 || i == 8 || i == 10)
> >> + cp++;
> >> +
> >> + buf[0] = *cp++;
> >> + buf[1] = *cp++;
> >> +
> >> + errno = 0;
> >> + uu[i] = strtoul(buf, NULL, 16);
> >> + if (errno)
> >> + return AVERROR(errno);
> >
> > How could this ever happen given that you have already checked that the
> > buffer only contains hex digits? And isn't using strtoul a bit overkill
> > anyway? I'd just check and parse this stuff in one loop.
> >
>
> Yeah, good point. It's based off libuuid's, which has some time/clock uuid
> handling
> between the two loops. I'll tidy it up in the next few days... hopefully...
>
> >> +/**
> >> + * @file
> >> + * UUID parsing and serialization utilities.
> >> + * The library treat the UUID as an opaque sequence of 16 unsigned bytes,
> > ^
> > s
>
> Fixed.
>
> >> +/**
> >> + * Parses a string representation of a UUID formatted according to IETF
> >> RFC 4122
> >> + * into an AVUUID. The parsing is case-insensitive. The string must be 37
> >> + * characters long, including the terminating NULL character.
> >
> > NUL, NULL is for pointers.
> >
>
> Changed.
>
> >> +/**
> >> + * Parses a string representation of a UUID formatted according to IETF
> >> RFC 4122
> >> + * into an AVUUID. The parsing is case-insensitive. The string must
> >> consist of
> >> + * 36 characters, i.e. `in_end - in_start == 36`
> >> + *
> >> + * @param[in] in_start Pointer to the first character of the string
> >> representation
> >> + * @param[in] in_end Pointer to the character after the last character
> >> of the
> >> + * string representation. That memory location is
> >> never
> >> + * accessed
> >> + * @param[out] uu AVUUID
> >> + * @return A non-zero value in case of an error.
> >> + */
> >> +int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID
> >> uu);
> >
> > This sounds like in_end is actually redundant. Does retaining it improve
> > extensibility?
> >
>
> I believe so. The main difference is av_uuid_parse_range() can handle non
> NUL-termiated
> strings. I can just remove the entire last sentence (or change "must" to
> "should").
I have modified the documentation in v3 of the patchset to emphasize
that `in_end - in_start != 36` results in an error.
>
> >> +
> >> +/**
> >> + * Serializes a AVUUID into a string representation according to IETF RFC
> >> 4122.
> >> + * The string is lowercase and always 37 characters long, including the
> >> + * terminating NULL character.
> >> + *
> >> + * @param[in] uu AVUUID
> >> + * @param[out] out Pointer to a array of no less than 37 characters.
> > ^
> > n
> >
>
> Fixed.
>
>
> >> +/**
> >> + * Sets a UUID to nil
> >> + *
> >> + * @param[in,out] uu UUID to be set to nil
> >> + */
> >> +void av_uuid_nil_set(AVUUID uu);
> >
> > Why are these three functions not static inline? Exporting them seems
> > like a waste.
> >
>
> No particular reason, it's easy enough to change.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
