Ben Chan | fad4a0b | 2012-04-18 15:49:59 -0700 | [diff] [blame] | 1 | Copyright (c) 2012 The Chromium OS Authors. All rights reserved. |
mukesh agrawal | 102a775 | 2011-07-28 14:57:14 -0700 | [diff] [blame] | 2 | Use of this source code is governed by a BSD-style license that can be |
| 3 | found in the LICENSE file. |
| 4 | |
| 5 | To keep the shill source code consistent, please follow the conventions below: |
| 6 | |
| 7 | - Use the Chromium Coding Style, as described at |
| 8 | http://www.chromium.org/developers/coding-style. |
| 9 | |
| 10 | If you use Emacs, the Google C Style mode will help you with the formatting |
| 11 | aspects of style. (Chromium Style generally follows Google Style). Get the |
| 12 | Emacs mode at |
| 13 | http://google-styleguide.googlecode.com/svn/trunk/google-c-style.el |
| 14 | |
| 15 | Note that we've deviated from the Chromium style in the following |
| 16 | ways. In these cases, follow the shill style, for consistency with |
| 17 | the rest of the shill code: |
| 18 | |
| 19 | - We denote pointer and reference variables by placing the '*' and '&' |
| 20 | adjacent to the variable name, rather than the type. E.g. |
| 21 | |
| 22 | void *bar |
| 23 | |
| 24 | rather than |
| 25 | |
| 26 | void* bar |
| 27 | |
| 28 | - <no other deviations documented yet> |
| 29 | |
| 30 | - When working with DBus::Variant: |
| 31 | - Read data via the appropriate named method, rather than depending on |
| 32 | implicit conversion. E.g., |
| 33 | |
mukesh agrawal | 6e27777 | 2011-09-29 15:04:23 -0700 | [diff] [blame] | 34 | ::DBus::Variant var; |
mukesh agrawal | 102a775 | 2011-07-28 14:57:14 -0700 | [diff] [blame] | 35 | int8 data = var.reader().get_byte(); |
| 36 | |
| 37 | rather than |
| 38 | |
mukesh agrawal | 6e27777 | 2011-09-29 15:04:23 -0700 | [diff] [blame] | 39 | ::DBus::Variant var; |
mukesh agrawal | 102a775 | 2011-07-28 14:57:14 -0700 | [diff] [blame] | 40 | int8 data = var; |
| 41 | |
| 42 | RATIONALE: The explicit version is only marginally longer than the |
| 43 | implicit version, and does not require the reader to understand C++ |
| 44 | conversion rules. |
| 45 | |
| 46 | - Where there is no named method, call the appropriate cast operator |
| 47 | explicitly. E.g. |
| 48 | |
mukesh agrawal | 6e27777 | 2011-09-29 15:04:23 -0700 | [diff] [blame] | 49 | ::DBus::Variant var; |
mukesh agrawal | 102a775 | 2011-07-28 14:57:14 -0700 | [diff] [blame] | 50 | vector<unsigned int> data = var.operator vector<unsigned int>(); |
| 51 | |
| 52 | RATIONALE: Calling the cast operator explicitly avoids conflicts with |
| 53 | constructors that might also be used to make the conversion. It also |
| 54 | avoids requiring that the reader understand C++ conversion rules. |
| 55 | |
mukesh agrawal | 6e27777 | 2011-09-29 15:04:23 -0700 | [diff] [blame] | 56 | - Write data via the appropriate named method. E.g., |
| 57 | |
| 58 | ::DBus::Variant var; |
| 59 | int16_t data; |
| 60 | var.writer().append_int16(data); |
| 61 | |
| 62 | rather than |
| 63 | |
| 64 | ::DBus::Variant var; |
| 65 | int16_t data; |
| 66 | var.writer() << data; |
| 67 | |
| 68 | RATIONALE: Similarly as for reading, the explicit version is only |
| 69 | marginally longer, and does not require the reader to understand |
| 70 | overload resolution. |
| 71 | |
| 72 | - Where there is no named method, write by using the stream |
| 73 | insertion operator. E.g. |
| 74 | |
| 75 | ::DBus::Variant var; |
| 76 | ::DBus::MessageIter writer; |
| 77 | map<string, string> data; |
| 78 | writer = var.writer(); |
| 79 | writer << data; |
| 80 | |
| 81 | RATIONALE: This case is somewhat unfortunate, because it's not as |
| 82 | clear as its analogue for reading. However, the alternative is to |
| 83 | duplicate the code of the stream insertion operator overloads. |
| 84 | |
| 85 | Note that the writer can't be omitted. E.g. |
| 86 | |
| 87 | ::DBus::Variant var; |
| 88 | map<string, string> data; |
| 89 | var.writer() << data; |
| 90 | |
mukesh agrawal | 1590839 | 2011-11-16 18:29:25 +0000 | [diff] [blame] | 91 | does not work. For an explanation of why the local variable |
| 92 | |writer| is needed, see the comment in |
| 93 | DBusAdaptor::ByteArraysToVariant. |
mukesh agrawal | 6e27777 | 2011-09-29 15:04:23 -0700 | [diff] [blame] | 94 | |
mukesh agrawal | 102a775 | 2011-07-28 14:57:14 -0700 | [diff] [blame] | 95 | - When deferring work from a signal handler (e.g. a D-Bus callback) to |
| 96 | the event loop, name the deferred work function by adding "Task" to |
| 97 | the name of the function deferring the work. E.g. |
| 98 | |
| 99 | void Modem::Init() { |
| 100 | dispatcher_->PostTask(task_factory_.NewRunnableMethod(&Modem::InitTask)); |
| 101 | } |
| 102 | |
| 103 | RATIONALE: The naming convention makes the relationship between the signal |
| 104 | handler and the task function obvious, at-a-glance. |
Ben Chan | fad4a0b | 2012-04-18 15:49:59 -0700 | [diff] [blame] | 105 | |
Ben Chan | 80326f3 | 2012-05-04 17:51:32 -0700 | [diff] [blame] | 106 | - C++ exceptions are not allowed in the code. An exception to this rule is |
| 107 | that try-catch blocks may be used in various D-Bus proxy classes to handle |
| 108 | DBus::Error exceptions thrown by the D-Bus C++ code. C++ exceptions should |
| 109 | be caught by const reference in general. |
| 110 | |
Ben Chan | fad4a0b | 2012-04-18 15:49:59 -0700 | [diff] [blame] | 111 | - When adding verbose log messages for debug purposes, use the SLOG marco and |
Christopher Wiley | b691efd | 2012-08-09 13:51:51 -0700 | [diff] [blame] | 112 | its variants (see logging.h for details). |
Ben Chan | fad4a0b | 2012-04-18 15:49:59 -0700 | [diff] [blame] | 113 | |
| 114 | - Choose the appropriate scope and verbose level for log messages. E.g. |
| 115 | |
| 116 | SLOG(WiFi, 1) << message; // for WiFi related code |
| 117 | |
| 118 | - Before defining a new scope, check if any existing scope defined in |
| 119 | scope_logger.h already fulfills the needs. |
| 120 | |
| 121 | - To add a new scope: |
| 122 | 1. Add a new value to the Scope enumerated type in scope_logger.h. |
| 123 | Keep the values sorted as instructed in the header file. |
| 124 | 2. Add the corresponding scope name to the kScopeNames array in |
| 125 | scope_logger.cc. |
| 126 | 3. Update the GetAllScopeNames test in scope_logger_unittest.cc. |
mukesh agrawal | bebf1b8 | 2013-04-23 15:06:33 -0700 | [diff] [blame] | 127 | |
| 128 | - When adding externally visible (i.e. via RPC) properties to an object, |
| 129 | make sure that a) its setter emits any change notification required by |
| 130 | Chrome, and that b) its setter properly handles no-op changes. |
| 131 | |
| 132 | Test that the property changes are handled correctly by adding test |
| 133 | cases similar to those in CellularServiceTest.PropertyChanges, and |
| 134 | CellularServiceTest.CustomSetterNoopChange. |