RE: [PATCH v2 6/7] udf: Remove struct ustr as non-needed intermediate storage

From: Andrew Gabbasov
Date: Mon Jan 11 2016 - 08:33:17 EST


Hi Jan,

Thank you for your review!
And sorry for a delay with response, we had several days of holidays.

> -----Original Message-----
> From: Jan Kara [mailto:jack@xxxxxxx]
> Sent: Monday, January 04, 2016 3:33 PM
> To: Gabbasov, Andrew
> Cc: Jan Kara; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 6/7] udf: Remove struct ustr as non-needed
intermediate storage

[skipped]

> >
> > - ocu[length - 1] = (uint8_t)u_len + 1;
> > - return u_len + 1;
> > + return u_len;
>
> It seems you removed setting of the length in the resulting CS0 string.

Yes, and it was done deliberately.
udf_name_to_CS0 and its caller udf_put_filename functions are used for
writing File Identifier fields only, which are not "dstrings", that is
not containing the length in last byte. The last byte with the length
from these functions would not be copied to filesystem or used in any
other way.

[skipped]

> > - ret = udf_translate_to_linux(dname, dlen,
> > - filename->u_name, filename->u_len,
> > - unifilename->u_name,
unifilename->u_len);
> > + ret = udf_translate_to_linux(dname, dlen, filename, dlen, sname,
slen);
>
> Umm, but here you pass CS0 name to udf_translate_to_linux() including
> beginning encoding character which didn't happen before your patch.
> So in case we have to compute CRC it will be different.

Yes, you are correct, this is a mistake (introduced when splitting
the original single patch). I will correct it in version 3 of the patches
(I will use sname + 1 and slen - 1).
Thank you for catching it!

I will prepare an update soon.

Thanks.

Best regards,
Andrew