[haiku-bugs] Re: [Haiku] #8449: TODO taking read only volumes into account in src/add-ons/kernel/file_systems/ntfs/fs_func.c
- From: "bonefish" <trac@xxxxxxxxxxxx>
- Date: Sun, 08 Apr 2012 14:38:13 -0000
#8449: TODO taking read only volumes into account in src/add-
ons/kernel/file_systems/ntfs/fs_func.c
---------------------------------+----------------------------
Reporter: kag_anil | Owner: 3dEyes
Type: enhancement | Status: new
Priority: normal | Milestone: R1
Component: File Systems/NTFS | Version: R1/alpha3
Resolution: | Keywords: NTFS, gsoc2012
Blocked By: | Blocking:
Has a Patch: 1 | Platform: All
---------------------------------+----------------------------
Comment (by bonefish):
- Camel case for local variables was fine. What didn't and still doesn't
agree with the coding style is the use of uncommon abbreviations.
`readOnlyCheck` would be correct. And the helper function's parameter
should be `device`, not `dev`.
- More coding style: `ioctl()` returns an int. An explicit check to
convert it to a bool should be used in the `if` condition (`ioctl(...) ==
0`).
- Helper function:
- Should be static.
- `B_READ_ONLY_DEVICE` is a flag. Returning it through a `status_t` is
unusual to say the least. Since you're not interested in the error code
anyway, `check_read_only()` should be named `is_read_only()` (or
`is_device_read_only()`) and return a bool. That also makes the local
variable in `fs_mount()` superfluous.
- The function should return early in case `open()` fails. That saves
one if level.
- `check` should be bool and named `isReadOnly`. That saves the
innermost if.
- While comments on the same line after the actual code are not
disallowed per se, they should generally be avoided (cf. the coding
guidelines). In this case the three comments don't add any value anyway
and can just be omitted.
--
Ticket URL: <http://dev.haiku-os.org/ticket/8449#comment:5>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.
Other related posts:
