[FFmpeg-devel] [PATCH] avio: add avio_get_str as a replacement for get_strz

[Ronald S. Bultje]

Hi,

On Tue, Mar 1, 2011 at 3:38 PM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de> wrote:
> On Tue, Mar 01, 2011 at 06:33:56PM +0100, Anton Khirnov wrote:
>> ?/**
>> + * Read a UTF-8 string from pb. The reading will terminate when either
>> + * a NULL character was encountered or maxlen bytes have been read.
>> + *
>> + * @return number of bytes read (is always <= maxlen).
>> + */
>> +int avio_get_str(AVIOContext *pb, int maxlen, char *buf, int buflen);
>
> 1) If you already say UTF-8 string you should definitely use uint8_t as type.

Why? libc uses char* for UTF8.

> 2) The mention of UTF-8 is quite questionable since the code does not care one
> ? bit about UTF-8 (e.g. it doesn't try to avoid storing partial encodings in the
> ? destination).
>
>> ?char *get_strz(AVIOContext *s, char *buf, int maxlen)
>> ?{
>> - ? ?int i = 0;
>> - ? ?char c;
>> -
>> - ? ?while ((c = avio_r8(s))) {
>> - ? ? ? ?if (i < maxlen-1)
>> - ? ? ? ? ? ?buf[i++] = c;
>> - ? ?}
>> -
>> - ? ?buf[i] = 0; /* Ensure null terminated, but may be truncated */
>> -
>> + ? ?avio_get_str(s, INT_MAX, buf, maxlen);
>> ? ? ?return buf;
>> ?}
>
>> +int avio_get_str(AVIOContext *s, int maxlen, char *buf, int buflen)
>> +{
>> + ? ?int ret = 0;
>> +
>> + ? ?while (ret < maxlen) {
>> + ? ? ? ?char c = avio_r8(s);
>> + ? ? ? ?ret++;
>> + ? ? ? ?if (!c)
>> + ? ? ? ? ? ?break;
>> + ? ? ? ?if (ret < buflen - 1)
>> + ? ? ? ? ? ?*buf++ = c;
>> + ? ?}
>> + ? ?*buf = 0;
>> + ? ?return ret;
>
>
> How about something like this instead:
>
> int i;
> av_assert(buflen >= 0 && maxlen >= 0);
> // reserve 1 byte for terminating 0
> buflen = FFMIN(buflen - 1, maxlen);
> for (i = 0; i < buflen; i++)
> ? ?if (!(buf[i] = avio_r8(s)))
> ? ? ? ?return i + 1;
> // note: your code currently is wrong for that case
> if (buflen)
> ? ?buf[i] = 0;
> for (; i < maxlen; i++)
> ? ?if (!avio_r8(s))
> ? ? ? ?return i + 1;
> return maxlen;

Looks a little nicer indeed. Can anyone (maybe Anton) send a
git-formatted patch that does something like this? Also, Reimar, if
you don't mind, I can commit this with Anton as author and then your
patch (hopefully you'll send one git-formatted) right on top, so that
authorship is properly attributed.

Ronald