BIND 10 #1206: Create datasource factory

[BIND 10 Development]

#1206: Create datasource factory
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  stephen                            |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20111011
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  4
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => jelte


Comment:

 Reviewing version 5e14c4caafaa44b92134c5df01b726f435f46845

 '''src/lib/datasrc/Makefile.am'''[[BR]]
 Just a comment on the file names - the names of the memory data source
 files in the separate DLL are of the form memory_datasrc.*, but those for
 the SQLite3 data source are sqlite3_accessor.*.  On the other hand,
 sqlite3_datasrc - and static_datasrc are included in the main library.
 This is confusing.

 '''src/lib/datasrc/client.h'''[[BR]]
 Good description of !DataSourceClientContainer, thanks.  It really helps
 the understanding of the code.

 The description of the return value in getSym() should indicate that NULL
 will be returned if the symbols does not exist.

 '''src/lib/datasrc/factory.h'''[[BR]]
 There are a few #includes commented out.

 I assume that the constructor takes a std::string because this is
 something read from the configuration, whereas getSym takes a char*
 because the names of the symbols retrieved are literal character strings
 in the code?  (If so, a note in the getSym() header to this effect would
 be useful.)

 Perhaps want to add another exception - !DataSourceLibrarySymbolError - to
 distinguish between a failure to load a library and a failure to access a
 symbol in the library?

 '''src/lib/datasrc/factory.cc'''[[BR]]
 !DataSourceClientContainer constructor: Suggest using reinterpret_cast
 instead of the C-style casts when casting the returned symbol to the
 correct type.

 Question: in what directory is the DLL located?  The reason for this
 question is that I would see that there are two locations that would be in
 use.  The first is the library directory of the BIND 10 installation
 (which may be a directory specified on the "configure" command line).  The
 second is a site-specific directory in which local data sources are
 stored.  If the location of the latter were stored in the configuration
 database somewhere, and that were preserved across BIND 10 upgrades, it
 would be possible to upgrade BIND 10 without affecting the local data
 source.

 '''src/lib/datasrc/memory_datasrc.cc'''[[BR]]
 addError(): by the time the code gets here, "errors" should be non-null.
 (Especially as this is a function in the anonymous namespace and visible
 only to functions declared below it - and all paths through those
 functions create the error list.)  Under these circumstances, can't
 "errors->add()" be called directly from within the code?

 '''src/lib/datasrc/sqlite3_accessor.cc'''[[BR]]
 checkConfig() can also be placed in the anonymous namespace (as it is in
 memory_datasrc.cc) - there is no reason for it to be visible outside the
 module.

-- 
Ticket URL: <http://bind10.isc.org/ticket/1206#comment:10>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development