agfline / LibAAF

Library for Advanced Authoring Format (AAF) file reading.
GNU General Public License v2.0
28 stars 6 forks source link

code consolidation #26

Open x42 opened 9 months ago

x42 commented 9 months ago

https://github.com/agfline/LibAAF/blob/9171e402bb374ce9e545997fabd67498a20709c4/src/AAFIface/URIParser.c#L730-L810

Is there any reason why you don't just use sscanf (s, "%d.%d.%d.%d", &a, &b, c&, &d) == 4 here? or better yet just use libcurl for all methods in this file?

agfline commented 9 months ago

Well, I had this URIParser from another project and I thought I could reuse it here. This also keeps libaaf dependency free. Have you experienced any issue with it ? Note that the same logic applies to RIFFParser.

x42 commented 9 months ago

It just seems rather fragile, specifically with an int size in the API. Why not a size_t? (IMHO the function should do a NULL termination check, rather than requiring the caller to provide the correct length).

It is not something I'd wish to maintain, particularly since there are established and well maintained solutions out there that can be used with free/libre software. So I was curious why you wrote and included it here.

I understand that may be fun to write it to learn and understand..

The public functions uriDecodeString, uriIsIPv6 etc should have a libaaf_ prefix to prevent potential symbol conflicts.

Also you don't need a NULL check in uriFree and other places, free(3) handles NULL just fine, you can safely call free(NULL).