On Sun, 23 Sep 2012 18:49:24 +0400
Vyacheslav Dubeyko <slava@xxxxxxxxxxx> wrote:
> This patch adds functionality of manipulating by records in attributes tree.
Some minor things:
>
> ...
>
> +int hfsplus_attr_build_key(struct super_block *sb, hfsplus_btree_key *key,
> + u32 cnid, const char *name)
> +{
> + int len;
> +
> + memset(key, 0, sizeof(struct hfsplus_attr_key));
> + key->attr.cnid = cpu_to_be32(cnid);
> + if (name) {
> + len = strlen(name);
> + if (len > HFSPLUS_ATTR_MAX_STRLEN) {
> + printk(KERN_ERR "hfs: invalid xattr name's length\n");
> + return -EINVAL;
> + }
> + hfsplus_asc2uni(sb,
> + (struct hfsplus_unistr *)&key->attr.key_name,
> + HFSPLUS_ATTR_MAX_STRLEN, name, len);
> + len = be16_to_cpu(key->attr.key_name.length);
> + } else {
> + key->attr.key_name.length = 0;
> + len = 0;
> + }
> + /* The key_length (be16) doesn't summed in the lenght of whole key.
s/lenght/length/
Also, this sentence is rather hard to understand. Rephrase, please?
> + But the length of the string (be16) should be included in sum.
> + So, offsetof(hfsplus_attr_key, key_name) is a trick that give
> + right value. */
The usual layout style for multiline comments is
/*
* ...
* ...
*/
> + key->key_len =
> + cpu_to_be16(offsetof(struct hfsplus_attr_key, key_name) +
> + 2 * len);
> +
> + return 0;
> +}
> +
> +void hfsplus_attr_build_key_uni(hfsplus_btree_key *key,
> + u32 cnid,
> + struct hfsplus_attr_unistr *name)
> +{
> + int ustrlen;
> +
> + memset(key, 0, sizeof(struct hfsplus_attr_key));
> + ustrlen = be16_to_cpu(name->length);
> + key->attr.cnid = cpu_to_be32(cnid);
> + key->attr.key_name.length = cpu_to_be16(ustrlen);
> + ustrlen *= 2;
> + memcpy(key->attr.key_name.unicode, name->unicode, ustrlen);
> + /* The key_length (be16) doesn't summed in the lenght of whole key.
> + But the length of the string (be16) should be included in sum.
> + So, offsetof(hfsplus_attr_key, key_name) is a trick that give
> + right value. */
dittoes.
> + key->key_len =
> + cpu_to_be16(offsetof(struct hfsplus_attr_key, key_name) +
> + ustrlen);
> +}
> +
> +hfsplus_attr_entry *hfsplus_alloc_attr_entry(void)
> +{
> + hfsplus_attr_entry *entry;
> + entry = kmem_cache_alloc(hfsplus_attr_tree_cachep, GFP_KERNEL);
> +
> + return (entry) ? entry : NULL;
> +}
This is rather verbose. It could be
hfsplus_attr_entry *hfsplus_alloc_attr_entry(void)
{
return kmem_cache_alloc(hfsplus_attr_tree_cachep, GFP_KERNEL);
}
>
> ...
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
