From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 790BA429DA for ; Wed, 11 May 2022 15:02:40 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A190668B3F5; Wed, 11 May 2022 18:02:37 +0300 (EEST) Received: from out2.migadu.com (out2.migadu.com [188.165.223.204]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B888E68B33C for ; Wed, 11 May 2022 18:02:31 +0300 (EEST) Message-ID: <04826f35-e7dd-b74d-f684-37f4ca920a5f@zanevaniperen.com> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=zanevaniperen.com; s=key1; t=1652281351; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GXKGHG4S0RRN5AwQixCySl6iE/+qqPctIIWZFjz/dO0=; b=KCXS037cq3QNCTjeBq50eGUrL5l0OJriyNHOjwzCdIl1IePplAos12LdLQyRcfBtwzChca HBRFfS1VsZAQpfEyWf9XWYvqCSGXqyx9/ehzktiozcx3DWEIWJh0rI1my/K3yYMfDXInkC TaODGZtmJ4ShuYKSuYlSe/taJH6Cf8j2Rzk8J0VBxDq6GsAM41WJ2awTBypTd38nwMItAm 6PLmrCLzDrta2mJ/CYaVgm6IP3Rzyc0Ht7q+ZELTV5UIEg9gTlPC+bE3uA97uibbf4dUQd sDY9QNd7cBLq6zQXi4S2sQfyHg7lKAVU8hbdlePyYPMiH7Wdid1RZEASVe08XA== Date: Thu, 12 May 2022 01:02:27 +1000 MIME-Version: 1.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220424101409.95486-1-zane@zanevaniperen.com> <20220424101409.95486-2-zane@zanevaniperen.com> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Zane van Iperen In-Reply-To: X-Migadu-Flow: FLOW_OUT X-Migadu-Auth-User: zanevaniperen.com Subject: Re: [FFmpeg-devel] [PATCH v2 1/7] avutil/uuid: add utility library for manipulating UUIDs as specified in RFC 4122 X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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"). >> + >> +/** >> + * 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-request@ffmpeg.org with subject "unsubscribe".