Home | History | Annotate | Download | only in shill
      1 
      2 Copyright (C) 2012 The Android Open Source Project
      3 
      4 Licensed under the Apache License, Version 2.0 (the "License");
      5 you may not use this file except in compliance with the License.
      6 You may obtain a copy of the License at
      7 
      8      http://www.apache.org/licenses/LICENSE-2.0
      9 
     10 Unless required by applicable law or agreed to in writing, software
     11 distributed under the License is distributed on an "AS IS" BASIS,
     12 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
     13 See the License for the specific language governing permissions and
     14 limitations under the License.
     15 
     16 To keep the shill source code consistent, please follow the conventions below:
     17 
     18 - Use the Chromium Coding Style, as described at
     19   http://www.chromium.org/developers/coding-style.
     20 
     21   If you use Emacs, the Google C Style mode will help you with the formatting
     22   aspects of style. (Chromium Style generally follows Google Style). Get the
     23   Emacs mode at
     24   http://google-styleguide.googlecode.com/svn/trunk/google-c-style.el
     25 
     26 - When working with DBus::Variant:
     27   - Read data via the appropriate named method, rather than depending on
     28     implicit conversion. E.g.,
     29 
     30        ::DBus::Variant var;
     31        int8 data = var.reader().get_byte();
     32 
     33     rather than
     34 
     35        ::DBus::Variant var;
     36        int8 data = var;
     37 
     38     RATIONALE: The explicit version is only marginally longer than the
     39     implicit version, and does not require the reader to understand C++
     40     conversion rules.
     41 
     42   - Where there is no named method, call the appropriate cast operator
     43     explicitly. E.g.
     44 
     45     ::DBus::Variant var;
     46     vector<unsigned int> data = var.operator vector<unsigned int>();
     47 
     48     RATIONALE: Calling the cast operator explicitly avoids conflicts with
     49     constructors that might also be used to make the conversion. It also
     50     avoids requiring that the reader understand C++ conversion rules.
     51 
     52   - Write data via the appropriate named method. E.g.,
     53 
     54        ::DBus::Variant var;
     55        int16_t data;
     56        var.writer().append_int16(data);
     57 
     58     rather than
     59 
     60        ::DBus::Variant var;
     61        int16_t data;
     62        var.writer() << data;
     63 
     64     RATIONALE: Similarly as for reading, the explicit version is only
     65     marginally longer, and does not require the reader to understand
     66     overload resolution.
     67 
     68   - Where there is no named method, write by using the stream
     69     insertion operator. E.g.
     70 
     71        ::DBus::Variant var;
     72        ::DBus::MessageIter writer;
     73        map<string, string> data;
     74        writer = var.writer();
     75        writer << data;
     76 
     77     RATIONALE: This case is somewhat unfortunate, because it's not as
     78     clear as its analogue for reading. However, the alternative is to
     79     duplicate the code of the stream insertion operator overloads.
     80 
     81     Note that the writer can't be omitted. E.g.
     82 
     83        ::DBus::Variant var;
     84        map<string, string> data;
     85        var.writer() << data;
     86 
     87     does not work. For an explanation of why the local variable
     88     |writer| is needed, see the comment in
     89     DBusAdaptor::ByteArraysToVariant.
     90 
     91 - When deferring work from a signal handler (e.g. a D-Bus callback) to
     92   the event loop, name the deferred work function by adding "Task" to
     93   the name of the function deferring the work. E.g.
     94 
     95   void Modem::Init() {
     96     dispatcher_->PostTask(task_factory_.NewRunnableMethod(&Modem::InitTask));
     97   }
     98 
     99   RATIONALE: The naming convention makes the relationship between the signal
    100   handler and the task function obvious, at-a-glance.
    101 
    102 - C++ exceptions are not allowed in the code. An exception to this rule is
    103   that try-catch blocks may be used in various D-Bus proxy classes to handle
    104   DBus::Error exceptions thrown by the D-Bus C++ code. C++ exceptions should
    105   be caught by const reference in general.
    106 
    107 - When adding verbose log messages for debug purposes, use the SLOG marco and
    108   its variants (see logging.h for details).
    109 
    110   - Choose the appropriate scope and verbose level for log messages. E.g.
    111 
    112       SLOG(WiFi, 1) << message;  // for WiFi related code
    113 
    114   - Before defining a new scope, check if any existing scope defined in
    115     scope_logger.h already fulfills the needs.
    116 
    117   - To add a new scope:
    118     1. Add a new value to the Scope enumerated type in scope_logger.h.
    119        Keep the values sorted as instructed in the header file.
    120     2. Add the corresponding scope name to the kScopeNames array in
    121        scope_logger.cc.
    122     3. Update the GetAllScopeNames test in scope_logger_unittest.cc.
    123 
    124 - When adding externally visible (i.e. via RPC) properties to an object,
    125   make sure that a) its setter emits any change notification required by
    126   Chrome, and that b) its setter properly handles no-op changes.
    127 
    128   Test that the property changes are handled correctly by adding test
    129   cases similar to those in CellularServiceTest.PropertyChanges, and
    130   CellularServiceTest.CustomSetterNoopChange.
    131 
    132 - When performing trivial iteration through a container, prefer using
    133   range based for loops, preferably:
    134 
    135      for (const auto& element : container) {
    136 
    137   Remove "const" where necessary if the element will be modified during
    138   the loop.  Removal of the "const" and reference for trivial types is
    139   allowed but not necessary.
    140