On Thu 24-12-15 10:25:37, Andrew Gabbasov wrote:
> Although 'struct ustr' tries to structurize the data by combining
> the string and its length, it doesn't actually make much benefit,
> since it saves only one parameter, but introduces an extra copying
> of the whole buffer, serving as an intermediate storage. It looks
> quite inefficient and not actually needed.
>
> This commit gets rid of the struct ustr by changing the parameters
> of some functions appropriately.
>
> Also, it removes using 'dstring' type, since it doesn't make much
> sense too.
>
> Just using the occasion, add a 'const' qualifier to udf_get_filename
> to make consistent parameters sets.
Some comments below:
> @@ -211,15 +163,18 @@ static int udf_name_to_CS0(dstring *ocu, struct ustr
> *uni, int length,
> wchar_t uni_char;
> int u_len, u_ch;
>
> - memset(ocu, 0, sizeof(dstring) * length);
> + if (ocu_max_len <= 0)
> + return 0;
> +
> + memset(ocu, 0, ocu_max_len);
> ocu[0] = 8;
> max_val = 0xff;
> u_ch = 1;
>
> try_again:
> - u_len = 0;
> - for (i = 0; (i < uni->u_len) && ((u_len + 1 + u_ch) < length); i++) {
> - len = conv_f(&uni->u_name[i], uni->u_len - i, &uni_char);
> + u_len = 1;
> + for (i = 0; (i < str_len) && ((u_len + u_ch) <= ocu_max_len); i++) {
> + len = conv_f(&str_i[i], str_len - i, &uni_char);
> if (!len)
> continue;
> /* Invalid character, deal with it */
> @@ -236,41 +191,37 @@ try_again:
> }
>
> if (max_val == 0xffff)
> - ocu[++u_len] = (uint8_t)(uni_char >> 8);
> - ocu[++u_len] = (uint8_t)(uni_char & 0xff);
> + ocu[u_len++] = (uint8_t)(uni_char >> 8);
> + ocu[u_len++] = (uint8_t)(uni_char & 0xff);
> i += len - 1;
> }
>
> - 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.
> -int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
> +int udf_CS0toUTF8(uint8_t *utf_o, int o_len, const uint8_t *ocu_i, int i_len)
> {
> - return udf_name_from_CS0(utf_o, ocu_i, udf_uni2char_utf8);
> + return udf_name_from_CS0(utf_o, o_len, ocu_i, i_len,
> + udf_uni2char_utf8);
> }
>
> -int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> +int udf_get_filename(struct super_block *sb, const uint8_t *sname, int slen,
> uint8_t *dname, int dlen)
> {
> - struct ustr *filename, *unifilename;
> + uint8_t *filename;
> int (*conv_f)(wchar_t, unsigned char *, int);
> int ret;
>
> if (!slen)
> return -EIO;
>
> - filename = kmalloc(sizeof(struct ustr), GFP_NOFS);
> + if (dlen <= 0)
> + return 0;
> +
> + filename = kmalloc(dlen, GFP_NOFS);
> if (!filename)
> return -ENOMEM;
>
> - unifilename = kmalloc(sizeof(struct ustr), GFP_NOFS);
> - if (!unifilename) {
> - ret = -ENOMEM;
> - goto out1;
> - }
> -
> - udf_build_ustr_exact(unifilename, sname, slen);
> if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
> conv_f = udf_uni2char_utf8;
> } else if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP)) {
> @@ -278,21 +229,17 @@ int udf_get_filename(struct super_block *sb, uint8_t
> *sname, int slen,
> } else
> BUG();
>
> - ret = udf_name_from_CS0(filename, unifilename, conv_f);
> + ret = udf_name_from_CS0(filename, dlen, sname, slen, conv_f);
> if (ret < 0) {
> udf_debug("Failed in udf_get_filename: sname = %s\n", sname);
> goto out2;
> }
>
> - 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.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
