Re: dumpcdat.c

From: Christian Holm Christensen (cholm@hehi03.nbi.dk)
Date: Wed Apr 05 2000 - 17:16:26 EDT

  • Next message: Konstantin Olchanski: "disk space on pii3:/home"

    Hi Konstantin et al, 
    
    On Wed, 5 Apr 2000 15:49:50 -0400 Konstantin Olchanski <olchansk@ux1.phy.bnl.gov> wrote:
    > 
    > > ... Now the reason why everything works out fine in the end,
    > > is because "strtol()" _does_ return a "int", but suppose that it
    > > actually returned a pointer, ...
    > 
    > strtol() actually returns a "long".
    
    Ups, of course you're right Konstantin ("l" for "long", not "int"). 
     
    > Such code would break on computers where "int != long", such as Alphas
    > running DEC UNIX.
    
    Hmm. Perhaps I should clarify. It doesn't fail on a 64 bit machine,
    like a DEC UNIX on Alpha (I tried), and the reason is simple: you can
    assign from a "long" to an "int". Usually this gives you a compile
    time error, but since the compiler has already complained about the
    missing prototype, it will probably not complain about a msiing type
    cast. 
     
    > I missed part of this discussion, but strtol() is defined
    > in stdlib.h and in my exerience it is a safe header file to include,
    > compatibility-wise.
    
    Ups. I made a mistake once again - sorry! The missing prototype
    wasn't for "int strtol(const char*, char**, int)" but 
    "int strncpy(char*, const char*, size_t)", which is decalared in
    "string.h". So the actual compiler warning was:
    
      dumpcdat.c:192: warning: implicit declaration of function `int strncpy(...)'
      
    My mistake. Anyway, the same arguments apply. Now I think Konstantin
    will agree with me, when I say that the "string.h" is also a safe
    header, compatiblity-wise. 
    
    Now that we are on the subject of compatibility, I'd like to draw
    Konstantins attention to the function 
    "static std::string KOsprintf(const char* format,...)" in
    "brat/raw/inc/KOsprintf.h". In that finction you use "vsnprintf()",
    which isn't exactly portable. On Digital Unix, a similar function is
    "_IO_vsnprintf()". Why don't you just use "vsprintf()" instead?. You
    don't really use the "size_t" argument to "vsnprintf()", at least not
    in a way that can't be modeled just as well with "vsprintf()". 
    
    Now, I do appricate the highly architecture-dependent implementations
    of the variadic functions, like "vsprintf", "printf", and so on, since
    they depend on the stack layout etc. In shoort, I'm not blind to that
    fact, that it may be impossible to be a 100% compatible on this. 
    
    Now, I'm not out to get you Konstantin, but I do have one more comment
    on one of the "KO..." files. This is about
    "brat/raw/inc/KOexception.h". I fail to see the point of this class,
    sorry but there you go. In ANSI C++ there is an equivilant ABC
    called "exception", so why invent the bicycle all over. Also, I've
    created one ABC "BrException", and three children "BrWarning",
    "BrError", and "BrFatal", all of which I've used extensively. The nice
    thing about these classes IMHO, is that the fit well into the scheme
    of ROOT/BRAT. 
    
    I'll soon (tomorrow I think) commit a class to the CVS rep, called
    "BrDetectorList". This is a list of detectors, and thier associated
    names and types, as well as a bit mask representing lit detectors. You
    (Konstantin) might find this useful somewhere in the Raw data handling
    (like BrRawInput perhaps). Anyway, it's there for the taking. 
    
    I guess lying in bed, being sick makes you think on al sorts of
    strange things, so I have a few more things to say - sorry! 
    
    In "BrEventIO", an object called "BRAHMS ROOT I/O File". I would
    prefer if this could be renamed to "BRAHMS ROOT IO File", since the
    '/' makes it impossible to use "TFile::Get()" to get the object (ROOT
    thinks it's a directory seperator). Also, could the "BrIOModule" class
    have a method to get the top most directory in the file opened, like 
    
      virtual TDirectory* BrIOModule::GetDirectory(void) const =0; 
      virtual TDirectory* BrEventIO::GetDirectory(void) const { 
         return fROOTFile; }
      virtual TDirectory* BrGeantInput::GetDirectory(void) const { 
         return 0; }
    
    In that way, It'd be possible to put histograms, etc., into the same
    file as the events.
    
    A question: Why doesn't "BrIOModule", and "BrModuleContainer" inherit
    from "BrModule". In that way, one could have nested modules, like 
    
      topContainer
        |
        +-geantInput
        |
        +-digitizeContainer
        |  |
        |  +-tof1Digitize
        |  |
        |  ...
        | 
        +-eventIO
    
    Actually I think this would make sense even more for the
    reconstruction applications. Even if nested constainers isn't desired,
    it still would make sense for "BrIOModule" and "BrModuleContainer" to
    inherit from "BrModule". I think these are trivial changes, and I'll
    gladly make them if everyone thinks it's a good idea. 
    
    As always, this mail got longer then I intended - sorry. Anyway,
    cheers to all of you, and bottoms up!
    
    Christian  -----------------------------------------------------------
    Holm Christensen                             Phone:  (+45) 35 35 96 91 
      Sankt Hansgade 23, 1. th.                  Office: (+45) 353  25 305 
      DK-2200 Copenhagen N                       Web:    www.nbi.dk/~cholm    
      Denmark                                    Email:       cholm@nbi.dk
     
    



    This archive was generated by hypermail 2b29 : Wed Apr 05 2000 - 17:18:25 EDT