BIND 10 #2172: Add support for OS X in b10-showtech

BIND 10 Development do-not-reply at isc.org
Wed Aug 8 00:03:05 UTC 2012


#2172: Add support for OS X in b10-showtech
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  jinmei
                       Type:         |                Status:  reviewing
  defect                             |             Milestone:
                   Priority:         |  Sprint-20120821
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  Unclassified                       |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  3      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 '''sysinfo.py'''

 - (although not specific to this branch) I don't think it a good
   practice to have Linux specific notions like mem_cached,
   mem_buffers, or get_platform_distro() at the top level.  It seems to
   be a typical example of the "the world is Linux" mindset.  IMO we
   should either introduce a higher abstraction for these that can
   apply more system independently or move them to Linux specific code.

 - The implementation seems to share much of the code with the FreeBSD
   version.  I'd unify them as much as possible based on the DRY
   principle.

 - at least on my MBP kern.smp.active isn't available:
 {{{
 % sysctl kern.smp.active
 second level name smp in kern.smp.active is invalid
 }}}
   and I don't even know if it's possible to activate/deactivate an SMP
   support on MacOS X.

 - I don't think it a good idea to have a hardcoded page_size
   default.  If it's not known (which should be very rare or indicate
   we need to update the parser script), IMO we should also leave
   parameters that depend on it unknown.

 - Likewise (and although it's not specific to the MacOS version), I'd
   generally avoid using magic numbers like "-1" to represent
   "unknown", "unavailable", etc, because magic numbers make the intent
   of the code more vague and can often lead to unexpected bugs (e.g.)
   as a result of automatic conversion.  Python has a perfection
   representation for this type of concepts: None.  I'd have
   get_mem_free, etc return None when the value is unknown and convert
   it to corresponding string at the main program, or we could simply
   have get_xxx return the corresponding string if we only use them for
   printing purposes.

 - The `_mem_free` value doesn't match the corresponding "Free" size
   shown in Activity Monitor.  It looks like we should use the sum of
   'Pages free' and 'Pages speculative' instead of only the former.
   See also:
 http://www.opensource.apple.com/source/xnu/xnu-1456.1.26/osfmk/mach/vm_statistics.h

 - hw.physmem doesn't match the actual available memory, at least on my
   MBP.  hw.memsize seems to be the one that should be used.  See:
   http://stackoverflow.com/questions/583736/determine-physical-mem-size-
 programmatically-on-osx

 '''sysinfo_test.py'''
 - `_my_osx_os_sysconf`: what's the magic number of 91?  Arbitrary
   chosen?  As a magic number hater, I'd rather define a constant
   variable with commenting the intent and use the variable throughout
   the file.  Also, using the same constants for different systems is
   probably not a good idea anyway as we wouldn't be able to catch some
   type of regression.

 - `_my_osx_subprocess_check_output`: this generally seems to be the
   same as that for FreeBSD.  If we unify the main code (see above),
   it's probably better to unify the test case, too.  Or, if we want to
   separate the two test cases (which is not necessarily a bad idea),
   it's probably better to use different faked constants.

 - `_my_osx_subprocess_check_output`: maybe related to the previous
   point, 'daemon' doesn't seem to be very compatible with OS X.
 {{{#!python
     if command == ['hostname']:
         return b'daemon.example.com\n'
 }}}

 - test_sysinfo_osx: I've fixed a trivial typo in a comment.  And, this
   seems to suggest we might rather unify these test_xxx's and switch
   from it depending on the system type, rather than repeating the same
   logic for every case.

 - test_sysinfo_osx: not really specific to this branch, but it'd be
   safer to restore the original functions in tearDown().

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


More information about the bind10-tickets mailing list