[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-
   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(...) ==
  - 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: