[haiku-bugs] Re: [Haiku] #2657: Implement XSI shared memory <sys/shm.h>

#2657: Implement XSI shared memory <sys/shm.h>
---------------------------+------------------------------
  Reporter:  kaliber       |        Owner:  emitrax
      Type:  enhancement   |       Status:  new
  Priority:  normal        |    Milestone:  R1
 Component:  System/POSIX  |      Version:  R1/Development
Resolution:                |     Keywords:
Blocked By:                |  Has a Patch:  1
  Platform:  All           |     Blocking:
---------------------------+------------------------------

Comment (by bonefish):

 Thanks for the patch. I haven't looked too closely yet, but it seems a
 good start. There's a whole bunch of coding style issues, though. Just
 pointing out a few:
  - Two blank lines between groups of unrelated stuff -- e.g. before and
 after `#include`, `#define`, variable declaration, etc. blocks and between
 functions. Never within functions or class definitions, though (max. one
 blank line there). No blank line between copyright header and header
 guard.
  - No blank line at the beginning or end of blocks
 (`SharedMemoryEntry::HasPermission()`).
  - Don't indent/align with anything but tabs. Only exception is in class
 definitions when e.g. the type needs more space than there is in its
 column. Then a single space can be used to separate it from the next
 column.
  - Don't indent names in local variable definitions.
  - No spaces after C cast operator (wrong: `(int) fArea`).
  - No space between function name and opening parenthesis (also
 `new(std::nothrow)`), but single space between `if`/`switch`/... and
 opening parenthesis.
  - Always name parameters in function/method prototypes.
  - Don't use uncommon abbreviations in identifiers. E.g. `addr` should be
 `address`, `fDs` should be `fData` or `fDefinition` or whatever that might
 stand for.
  - Use a consistent pointer style, preferably `Foo* foo`.
  - No unnecessary parentheses (wrong: `fDs.shm_perm.mode = (flags &
 0x01ff);`).
  - In function definitions define variables where they are first used, not
 at the beginning of the function. Whenever possible combine definition and
 initialization (`status_t status = ...`).
  - `if (fArea < B_OK)`: Compare with `< 0` instead, when values `>= 0` are
 valid. Use the stricter comparison `!= B_OK`, when the value is supposed
 to be an error code (i.e. `B_OK` or another error code, normally of type
 `status_t`).
  - Explicitly force bit check expressions to boolean (`(flags & IPC_CREAT)
 != 0`).
  - When wrapping an expression, the operator always goes to beginning of
 the next line.
  - Broken indentation in `SharedMemoryEntry::HasReadPermission()` and
 `_user_xsi_shmget()`.

 I haven't really looked at the implementation, but I noticed:
  - `_user_xsi_shmat()`: `raddr` needs to be copied out via
 `user_memcpy()`. Besides, it needs a better name. Also, it is recommended
 to prefix output parameters with `_`, e.g. `_mappedAddress` in this case.
  - Using fully locked kernel areas is not a good idea. The locking isn't
 necessary at all. But the bigger issue is that kernel address space is a
 rather limited resource. I'd only create a `VMCache` and map it into the
 user address spaces as needed, but not into the kernel address space at
 all.

-- 
Ticket URL: <http://dev.haiku-os.org/ticket/2657#comment:16>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.

Other related posts: