lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 4 Apr 2011 11:38:42 +0200
From:	Sedat Dilek <sedat.dilek@...glemail.com>
To:	dedekind1@...il.com
Cc:	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Adrian Hunter <adrian.hunter@...ia.com>,
	Jens Axboe <jaxboe@...ionio.com>
Subject: Re: [PATCH] ubifs: Use empty_iops and empty_fops consistently

On Mon, Apr 4, 2011 at 9:04 AM, Artem Bityutskiy <dedekind1@...il.com> wrote:
> On Mon, 2011-04-04 at 01:55 +0200, Sedat Dilek wrote:
>> Follow the nomenclature as we did for empty_aops and rename
>> to empty_iops and empty_fops.
>>
>> This patch is on top of linux-2.6-block.git#for-linus.
>>
>> Signed-off-by: Sedat Dilek <sedat.dilek@...il.com>
>> CC: Artem Bityutskiy <dedekind1@...il.com>
>> CC: Adrian Hunter <adrian.hunter@...ia.com>
>> CC: Jens Axboe <jaxboe@...ionio.com>
>> ---
>>  fs/ubifs/xattr.c |    8 ++++----
>>  1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
>> index 3299f46..16f19f5 100644
>> --- a/fs/ubifs/xattr.c
>> +++ b/fs/ubifs/xattr.c
>> @@ -80,8 +80,8 @@ enum {
>>       SECURITY_XATTR,
>>  };
>>
>> -static const struct inode_operations none_inode_operations;
>> -static const struct file_operations none_file_operations;
>> +static const struct inode_operations empty_iops;
>> +static const struct file_operations empty_fops;
>>
>>  /**
>>   * create_xattr - create an extended attribute.
>> @@ -131,8 +131,8 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
>>
>>       /* Re-define all operations to be "nothing" */
>>       inode->i_mapping->a_ops = &empty_aops;
>> -     inode->i_op = &none_inode_operations;
>> -     inode->i_fop = &none_file_operations;
>> +     inode->i_op = &empty_iops;
>> +     inode->i_fop = &empty_fops;
>>
>>       inode->i_flags |= S_SYNC | S_NOATIME | S_NOCMTIME | S_NOQUOTA;
>>       ui = ubifs_inode(inode);
>
> Well, this re-naming looks strange for me. I could understand if you had
> a global exported empty_*ops which you could share and shrink the .data
> size a bit. But this re-naming just looks useles, if you take into
> account the breakage and the men-hours we spend to write e-mails to this
> thread - really did not worth it. Or I miss something?
>

Sorry for having flooded your inbox.
The last days might be the first time I noticed UBIFS at all and
looked into its code.
I did so because I fell over a build-error when pulling
linux-block-2.6.git#for-linus into linux-next (next-20110404).

The current code in linux-next (next-20110404) looks in the meantime like this:

[ fs/ubifs/xattr.c ]
...
static const struct inode_operations none_inode_operations;
static const struct file_operations none_file_operations;
...
        /* Re-define all operations to be "nothing" */
        inode->i_mapping->a_ops = &empty_aops;
        inode->i_op = &none_inode_operations;
        inode->i_fop = &none_file_operations;
...

First seeing this was strange to me, so you try via code-grep-ping to
understand the correlations in the complete fs-tree.
So, let's do some code-grep:

$ egrep 'empty_aops|mpty_iops|empty_fops' -r fs/ include/linux/
fs/ubifs/xattr.c:       inode->i_mapping->a_ops = &empty_aops;
fs/open.c:      static const struct file_operations empty_fops = {};
fs/open.c:              f->f_op = &empty_fops;
fs/inode.c:const struct address_space_operations empty_aops = {
fs/inode.c:EXPORT_SYMBOL(empty_aops);
fs/inode.c:     static const struct inode_operations empty_iops;
fs/inode.c:     static const struct file_operations empty_fops;
fs/inode.c:     inode->i_op = &empty_iops;
fs/inode.c:     inode->i_fop = &empty_fops;
fs/inode.c:     mapping->a_ops = &empty_aops;
fs/nilfs2/page.c:       mapping->a_ops = &empty_aops;
include/linux/fs.h:extern const struct address_space_operations empty_aops;

Hope you can understand now, why I did this.

It's up to you how you name your variables, function-names etc.

- Sedat -
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists