Changes in progress (was: Compiling for W...)

Developers
jim_monte
2006-05-30
2013-04-22
  • jim_monte
    jim_monte
    2006-05-30

    Transferred from Help list to here where
    it seems to better fit...

    Sean,

    I think we both agree that the number of
    changes I have made is getting large and
    merging with existing CVS source will become
    increasingly difficult as they grow. So I will
    complete the changes I am in the process of
    making, test them, and then look into how to
    submit the changes.

    A summary of the changes I am working on or
    have completed follows.

    - Elimination of compiler warnings from the
    Windows build. There are 4 remaining that I
    have intentionally left to look at more after I
    understand the overall package better.

    - Consistent const char **argv for TCL functions
    and function pointers. This change followed
    from the compiler warnings. A few may still
    be hiding somewhere, but this is essentially
    done. This was mostly accomplished by adding a
    huge number of const keywords, in the
    prototypes, definitions, and variables that
    referencing the stings being passed.
    However, there were a few instances
    where the strings were really changed. In all
    but one case, I did this the "right" way,
    reworking the functions to leave the strings
    alone instead of adding the overhead of an
    allocate + copy. In that last case, there was
    what seemed to be an unlikely situation that
    was much easier to handle by making a copy.

    - New timing facilities for Windows. The
    replacement uses QueryPerformanceCounter()
    and QueryPerformanceFrequency() for elapsed
    time and GetProcessTimes() for kernel and
    user times. The change should be transparent
    to the caller, except for getting the right
    times.

    - libbu modifications
    - A winutil file for utilities to work with
    Windows. This file currently has only a
    single function to change the '\' characters
    in a Windows path specification to the '/'
    characters used in Unix. This function
    was created because I noticed this conversion
    being done frequently with the same code
    copied and pasted. I made it general in that
    it can accept a size for the converted string
    so it need not be null terminated and returns
    the size of the string since that information
    can essentially be found for free and may
    be of use to the caller. The ability to
    work with strings lacking a null termination
    can be useful for both performance reasons
    and to allow a string to remain a
    const char *. I generally perfer to keep
    track of string sizes myself to avoid things
    like strlen().

    - enhanced bu_bomb() to allow variable
    arguments in message.

    - bu_vls upgrades. I noticed that in a
    significant number of instances, the vls
    facility is used to build a small string
    that is then discarded. With the current
    version, this will always require an
    allocation and free. To avoid this, a small
    buffer was added to the end of the existing
    structure, and the init call points the
    string pointer to this buffer. The remaining
    bu_vls functions were modified to make
    the change transparent to callers of bu_vls.
    While making the changes, I noticed that
    the bu_vls_trimspace function was implemented
    inefficiently with multiple function calls to
    process a single character. So this funciton
    was rewritten as well.
    - Improved efficiency in help messages. The
    few instances I had mentioned where the
    bu_vls facility was used to create a copy
    of a string that does not change were
    only the tip of an iceberg. I am under water
    now getting to the bottom. I have also
    reformatted to some extent when making these
    changes to conform with the coding standards
    given because I had the alternatives of putting
    in fixes that did not conform with the
    standards, adding code that did conform that
    was surronded by non-conforming code so that
    it looked wrong, or fixing the wole thing to
    conform. The conventions followed were as
    in HACKING with two additions that I prefer.
    Braces are always used to group loops and
    ifs, with the exception of "else if". This
    eases debugging and reduces the chances of
    errors when another line is added to existing
    code. Finally, the code is limited to a line
    length of 80 characters, treating the tabs as
    if they had their expanded size. This is
    easier to read than lines wrapping around
    and makes an occasional printing look much
    better.

    - Occasional efficiency improvements. One I
    recall offhand is replacing something like
    if (strlen(s) == 0) with if (*s == '\0') to
    avoid the function call, which is doing noting
    useful once it finds that the length is at
    least 1.

    - Project file fixes. There were problems
    with the dependencies that caused a linker
    warining that was strange and difficult to
    identify. Also the post-build steps were
    improved to echo information to the console
    and to conditionally rebuild the destination
    directories if they do not exist. One
    additional thing I want to do in this iteration
    is to separate the intermediate files (.obj,
    etc. residing in directories Debug and
    Release) into their own directory. This will
    keep the directories holding the project
    files clean, which will make it easier to move
    the project from one computer to another.

    - Others? I must be forgetting something here...

    No, I have never used IRC. I will have to
    look into that. I guess I still think in
    batch mode. BTW, I still am not sure what
    the differences between the Open Discussion
    and Help forums are since all the posts seem to
    be asking for some type of help, but replies to
    this message probably belong in the Developers
    forum, so I will look there.

    Jim

     
    • jim_monte
      jim_monte
      2006-05-30

      I had hoped to complete changes I had been
      working on and be ready to start testing them
      more thoroughly after the long weekend, but
      there was more work to do here than I had
      anticipated.  I found several more "char **argv"
      prototypes that needed to be converted to "const
      char **argv."  This was due to incomplete
      function and function pointer prototypes (e.g.,
      bad_fun()) and external function declarations
      within the .c file instead of headers.  This
      largly defeats the type checking provided by the
      headers since a compiler will accept whatever
      definition is given and the linker cannot do much
      checking.  (One exception is if a different
      linkage convention is used if it leads to
      different name decoration, but that is rarely
      of much help.)  It would be useful to change
      the code to consistently use headers for all
      external references, but I will not even think
      about doing it until my current batch of chages
      are integrated.

      Others changes that I recall working on are
      - Adding a bu_vls_memcpy() to assist with
      working on unterminated strings or string
      segments (which is useful when making the
      char **argv's const)
      - Closing some Windows handles when a process
      is created.  These had been left open, which
      would prevent the release of the resources
      associated with them until the program exits.
      That is not terrible if it is done only a few
      times, but the effects are cumulative.
      - Allowing an EDITOR variable to select the
      editor to use in the Windows version as the
      comments describe and as is allowed in Unix.
      (I prefer VIM over notepad for anything besides
      trivial editing.)  There is also the issue of
      the 2-character end-of-line indicator in Windows
      that does not appear to be addressed.  I will
      look into this further since I am still resolving
      a const char related to the editing.  BTW, that
      is the last of them!
      - Fixed the Windows version of printing a uid.
      Windows returns a text name which was being
      found OK but printed using a %d format.  This
      almost certainly was a copy and past from the
      Unix version that was not fully modified.
      - Cache the number of processors in Windows
      since it cannot be dynamically varied.
      - Cache the user name since it too is constant.
      - Others?  Probably.  Should I think of them,
      I will add them here for documentation purposes.

      Jim

       
    • jim_monte
      jim_monte
      2006-06-01

      Adding to the list...
      A new utility function for Windows,
      bu_log_win32e() that functions much like
      perror(), except it formats the message based
      on Windows error codes obtained using
      GetLastError() and takes a variable number
      of arguments to use for formatting the message.
      This will be useful when adding tests for
      failure to some of the calls which do not
      currently have them.

      Jim

       
    • Sean Morrison
      Sean Morrison
      2006-06-01

      Jim,

      The changes mostly sound great.  There's a lot of great work there that you've done that I'm anxious to get tested and integrated.  From the description, it sounds like most of the changes are completely acceptible modifications to have applied.

      The only one that raises a red flag is the bu_bomb() modification which will be highly dependant upon the nature of the change.  In general (though not strictly speaking), bu_bomb() is merely a grace saving attempt to abort somewhat cleanly for a situation that would otherwise be a segmentation violation, bus error, or some other catastrophic condition that requires an immediate halt.  As this includes conditions like being completely out of main or stack memory, bu_bomb() itself is implemented to function under minimal conditions.  It doesn't require any additional stack allocation, for example, nor does it directly write to any memory.  Even the call to fprintf() that it makes should probably not be using %s, though that is highly dependant upon the implementation of fprintf().

      If you're interested in communicating over IRC (which I do highly recommend), there's a lot of info for getting started at http://irchelp.org.  You basically install an IRC client and join the #brlcad channel on the irc.freenode.net network (port 6667).  As for an IRC client, there are lots -- xchat is rather popular on windows, there are other options available too.

      Batch mode submissions are workable, but they are certainly more "painful" as they can take a rather long time to review and integrate especially if there are any issues that require changes.  Smaller patches (for first-time or external contributions) and smaller CVS commits (for developers) are considerably easier to peer review and validate.  I'd rather see people commit directly to CVS whenever possible, but that's something that generally has to be discussed and set up via e-mail or IRC.

      As for the forums, it's usually easier to just think of them as exclusive categories on developer and help.   If it's developer related (whether asking for help or intended to be held as an open developer discussion or not), then it's developer forum discussion material.  If it's asking for assistance, it belongs in the help forum.  Otherwise, it's open discussion.

      Cheers!
      Sean

       
      • jim_monte
        jim_monte
        2006-06-02

        Sean,

        I am getting close to switching to something
        more like a "testing mode" so I can verify that
        things are working properly before I submit the
        changes.  This brings up the question of what
        is available for black-box testing to verify
        proper overall functionality.  I saw the
        benchmarks (although I have not looked at just
        what they cover and test yet) but nothing
        else.  Is there any existing set of tests that
        can be run?  There are hundreds of basic units
        written and waiting to be run.  Right?  Please
        say yes...

        bu_bomb() is interesting in a way to me because
        it is a different way to program.  When I
        write something from scratch, I never allow a
        called function terminate the program.  That
        is, should something bad happen, a function
        will always return an error code which the
        caller can use to determine how to proceed.
        Termination is always by the function that is
        the entry point (say main()), and unless there
        is a good reason, there is only one exit point.
        This gives the opportunity to attempt a recovery
        somewhere or at least to produce a nice traceback
        of messages from nested function calls if the
        problem is fatal.  The tradeoff is that every call that can fail needs to be tested, although I
        personally think it is worthwhile.  Most of the
        calls I have seen so far to bu_bomb() are made
        when something undesirable but not in itself
        fatal occurs, such as failure of a memory
        allocation or failure to open a file.

        As far as the actual proposed change to bu_bomb()
        it seems to be a fairly trivial change from
        fprintf() to vfprintf().

        Jim