Hi,
On 7 May 2014, at 17:25, Vyacheslav Dubeyko <slava@xxxxxxxxxxx> wrote:
> On Wed, 2014-05-07 at 16:55 +0100, Anton Altaparmakov wrote:
>
> [snip]
>>> +
>>> + mode = be16_to_cpu(entry.file.permissions.mode);
>>> + if (S_ISFIFO(mode))
>>> + type = DT_FIFO;
>>> + if (S_ISCHR(mode))
>>> + type = DT_CHR;
>>> + if (S_ISBLK(mode))
>>> + type = DT_BLK;
>>> + if (S_ISREG(mode))
>>> + type = DT_REG;
>>> + if (S_ISLNK(mode))
>>> + type = DT_LNK;
>>> + if (S_ISSOCK(mode))
>>> + type = DT_SOCK;
>>> +
>>
>> I would either use "switch"/"case" or "else if" instead of "if" otherwise you are doing all the checks for all the types even after you have found the right one - also place the DT_REG check first as that is the common case...
>>
>
> I believe that such example is good solution too:
>
> static unsigned char
> nilfs_filetype_table[NILFS_FT_MAX] = {
> [NILFS_FT_UNKNOWN] = DT_UNKNOWN,
> [NILFS_FT_REG_FILE] = DT_REG,
> [NILFS_FT_DIR] = DT_DIR,
> [NILFS_FT_CHRDEV] = DT_CHR,
> [NILFS_FT_BLKDEV] = DT_BLK,
> [NILFS_FT_FIFO] = DT_FIFO,
> [NILFS_FT_SOCK] = DT_SOCK,
> [NILFS_FT_SYMLINK] = DT_LNK,
> };
>
> What about likewise way?
Sure, that also works. My objection was purely because all the "if"s were being evaluated unnecessarily.
Best regards,
Anton
> Thanks,
> Vyacheslav Dubeyko.
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
University of Cambridge Information Services, Roger Needham Building
7 JJ Thomson Avenue, Cambridge, CB3 0RB, UK
--
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
