Is this a good habit?

Discussion in 'C Programming' started by Snis Pilbor, Jul 27, 2006.

  1. Snis Pilbor

    Snis Pilbor Guest

    Hi,

    It seems pretty common to return pointers to a static array, for
    example:

    char *capitalize( char *name )
    {
    static char buf[MAX_STRING_LENGTH];

    sprintf( buf, "%s", name );
    if ( *buf >= 'a' && *buf <= 'z' )
    *buf += 'A' - 'a';

    return buf;
    }

    But when I do something like this, I like to instead keep a static
    pointer and malloc it the first time, to save space in whatever bank
    static memory is stored in, since I have this vague idea that if too
    much space is used in static memory bad things will happen? So I do
    this instead:

    char *capitalize( char *name )
    {
    static char *buf;

    if ( !buf )
    {
    buf = malloc( MAX_STRING_LENGTH );
    if ( !buf ) return name;
    }

    sprintf( buf, "%s", name );
    if ( *buf >= 'a' && *buf <= 'z' )
    *buf += 'A' - 'a';

    return buf;
    }

    Here is how I analyze both methods as far as efficiency:

    * the 2nd version uses less space in delicate "static memory" (not sure
    what that's called)
    * the 1st is much faster on the first call
    * the 1st is slightly faster the other calls (assuming the first time,
    malloc worked)
    * the 1st does not assume the OS will do appropriate cleanup on exit,
    as the 2nd does
    * the 1st is quicker to type

    I wonder which method is really best. I suppose it depends on how big
    MAX_STRING_LENGTH is. I assume for instance one wouldn't want to
    casually declare multi-hundred-kilobyte static arrays?

    Thanks for any help =)
     
    Snis Pilbor, Jul 27, 2006
    #1
    1. Advertising

  2. Snis Pilbor wrote:
    > Hi,
    >
    > It seems pretty common to return pointers to a static array, for
    > example:
    >
    > char *capitalize( char *name )
    > {
    > static char buf[MAX_STRING_LENGTH];
    >
    > sprintf( buf, "%s", name );
    > if ( *buf >= 'a' && *buf <= 'z' )
    > *buf += 'A' - 'a';
    >
    > return buf;
    > }


    IMHO, Use of static variables inside functions is not a good idea. You
    will end up with code that is not thread safe. The function will
    misbehave if multiple concurrently running threads are invoking the
    function with static variables.

    --
    VisualEther Protocol Analyzer 1.0 -
    http:://www.EventHelix.com/VisualEther
    Visual protocol analysis and sequence diagram generation from Ethereal
     
    EventHelix.com, Jul 27, 2006
    #2
    1. Advertising

  3. Snis Pilbor

    Snis Pilbor Guest

    EventHelix.com wrote:
    > Snis Pilbor wrote:
    > > Hi,
    > >
    > > It seems pretty common to return pointers to a static array, for
    > > example:
    > >
    > > char *capitalize( char *name )
    > > {
    > > static char buf[MAX_STRING_LENGTH];
    > >
    > > sprintf( buf, "%s", name );
    > > if ( *buf >= 'a' && *buf <= 'z' )
    > > *buf += 'A' - 'a';
    > >
    > > return buf;
    > > }

    >
    > IMHO, Use of static variables inside functions is not a good idea. You
    > will end up with code that is not thread safe. The function will
    > misbehave if multiple concurrently running threads are invoking the
    > function with static variables.
    >


    Yes, of course, it goes without saying in either case one must exercise
    caution.
    IMHO one of the most beautiful things about C is that it allows you
    such flexibility. It does not assume you are a baby and hold your
    hand.
    If I wanted a language where various people told me "Oh you can't do
    that, it's too dangerous", I would program Java =) Thanks for the word
    of warning anyway, though.
    I will continue to return pointers to static arrays in time sensitive
    functions because I am, and those who work under me are, smart enough
    to know what we're doing. =)
     
    Snis Pilbor, Jul 27, 2006
    #3
  4. Snis Pilbor

    Morris Dovey Guest

    Snis Pilbor (in )
    said:

    | I will continue to return pointers to static arrays in time
    | sensitive functions because I am, and those who work under me are,
    | smart enough to know what we're doing. =)

    Hmm. Seems to me that for time-sensitive functions the 'smart' thing
    to do would be to use strcpy() to make the copy and do the
    single-character conversion from lower- to upper-case inline.

    --
    Morris Dovey
    DeSoto Solar
    DeSoto, Iowa USA
    http://www.iedu.com/DeSoto
     
    Morris Dovey, Jul 27, 2006
    #4
  5. Snis Pilbor <> wrote:

    > IMHO one of the most beautiful things about C is that it allows you
    > such flexibility. It does not assume you are a baby and hold your
    > hand.


    Well, some would disagree with you, but then again, some are bad C
    programmers :)

    > I will continue to return pointers to static arrays in time sensitive
    > functions because I am, and those who work under me are, smart enough
    > to know what we're doing. =)


    I don't have experience on embedded platforms (it sounds like you are
    working on one), but from other posts to this group it seems that
    dynamic memory rather than static memory is most at a premium. If you
    have limited automatic storage, and you do this sort of thing
    frequently, you might think about using a global union rather than
    several static arrays; that could save you some more space.

    --
    C. Benson Manica | I *should* know what I'm talking about - if I
    cbmanica(at)gmail.com | don't, I need to know. Flames welcome.
     
    Christopher Benson-Manica, Jul 27, 2006
    #5
  6. > It seems pretty common to return pointers to a static array, for
    >example:


    >char *capitalize( char *name )
    >{
    > static char buf[MAX_STRING_LENGTH];


    > sprintf( buf, "%s", name );
    > if ( *buf >= 'a' && *buf <= 'z' )
    > *buf += 'A' - 'a';


    > return buf;
    >}


    > But when I do something like this, I like to instead keep a static
    >pointer and malloc it the first time, to save space in whatever bank
    >static memory is stored in, since I have this vague idea that if too
    >much space is used in static memory bad things will happen? So I do
    >this instead:


    >char *capitalize( char *name )
    >{
    > static char *buf;


    > if ( !buf )
    > {
    > buf = malloc( MAX_STRING_LENGTH );
    > if ( !buf ) return name;
    > }


    > sprintf( buf, "%s", name );
    > if ( *buf >= 'a' && *buf <= 'z' )
    > *buf += 'A' - 'a';


    > return buf;
    >}


    >Here is how I analyze both methods as far as efficiency:


    >* the 2nd version uses less space in delicate "static memory" (not sure
    >what that's called)
    >* the 1st is much faster on the first call
    >* the 1st is slightly faster the other calls (assuming the first time,
    >malloc worked)
    >* the 1st does not assume the OS will do appropriate cleanup on exit,
    >as the 2nd does
    >* the 1st is quicker to type


    >I wonder which method is really best. I suppose it depends on how big
    >MAX_STRING_LENGTH is. I assume for instance one wouldn't want to
    >casually declare multi-hundred-kilobyte static arrays?


    I prefer the first form (Assuming the size of memory you are allocating is
    not huge - in which case you need to manage it much better than either
    scenario), because:

    - You are assuming there is more memory available on the heap than is
    available during program initialization (which may well be allocated from
    the system heap) - Unless you have information about the specific system
    and run time environment (which makes it not a C question), this is probably
    not a valid assumption.

    - You never release the dynamically allocated memory, so assuming the
    program eventually executes all of it's functions, your memory requirements
    will be the same (in fact slightly higher because...):

    - There is overhead in using the heap, block headers etc. which slightly
    increase the memory usage for the heap based solution.

    - Each malloc results in a allocation block on the heap, possibly fragmenting
    memory (interleaved mallocs could be occuring within the same or a different
    program. The static allocations will be resolved at compile/link time and
    will (likely) occur within a single block which is organized and allocated at
    program startup.

    - Allocating from the heap takes cycles, testing that it has been allocated
    also takes cycles (on each call).

    - In this particular example, a shortage of memory will cause the program to
    begin to behave erraticaly {capitalize() stops working}. Better to either
    have the program fail to load if there is insufficient memory, or provide
    better fault handling.

    - As you pointed out, this is "untidy" in that the program does not free
    the memory.

    - Someone else mentioned the unsuitability of static memory in certain
    concurrent processing applications, however your static pointer will
    not resolve that problem. This isn't really an issue relating to the
    question that was asked, as the runtime environment was not specified.

    Regards,
    Dave


    --
    Dunfield Development Services http://www.dunfield.com
    Low cost software development tools for embedded systems
    Software/firmware development services Fax:613-256-5821
     
    Dave Dunfield, Jul 27, 2006
    #6
  7. Snis Pilbor

    Eric Sosman Guest

    Snis Pilbor wrote:

    > Hi,
    >
    > It seems pretty common to return pointers to a static array, for
    > example:
    >
    > char *capitalize( char *name )
    > {
    > static char buf[MAX_STRING_LENGTH];
    >
    > sprintf( buf, "%s", name );
    > if ( *buf >= 'a' && *buf <= 'z' )
    > *buf += 'A' - 'a';
    >
    > return buf;
    > }


    As an aside, this code has some problems. You'll be in
    trouble if `name' has more than MAX_STRING_LENGTH-1 characters
    (which means the macro name is a bit misleading), sprintf() is
    a silly way to do strcpy(), and the manipulation of letter
    case is badly misguided -- use toupper() instead.

    > But when I do something like this, I like to instead keep a static
    > pointer and malloc it the first time, to save space in whatever bank
    > static memory is stored in, since I have this vague idea that if too
    > much space is used in static memory bad things will happen? So I do
    > this instead:
    >
    > char *capitalize( char *name )
    > {
    > static char *buf;
    >
    > if ( !buf )
    > {
    > buf = malloc( MAX_STRING_LENGTH );
    > if ( !buf ) return name;


    Whoa, Nellie! If returning the unmodified original is an
    acceptable outcome, why bother writing the function at all?

    > }
    >
    > sprintf( buf, "%s", name );
    > if ( *buf >= 'a' && *buf <= 'z' )
    > *buf += 'A' - 'a';
    >
    > return buf;
    > }
    >
    > Here is how I analyze both methods as far as efficiency:
    >
    > * the 2nd version uses less space in delicate "static memory" (not sure
    > what that's called)


    It probably uses less if sizeof(char*) < MAX_STRING_LENGTH.
    On the other hand, it uses more total space. The relative
    "delicacy" of static and dynamic memory can only be assessed
    in relation to the platform you're coding for; it's not a matter
    the language itself describes at all.

    > * the 1st is much faster on the first call


    Probably -- although, again, speed is not a language issue,
    but a characteristic of the platform.

    > * the 1st is slightly faster the other calls (assuming the first time,
    > malloc worked)


    I see no compelling reason to think so.

    > * the 1st does not assume the OS will do appropriate cleanup on exit,
    > as the 2nd does
    > * the 1st is quicker to type


    You've overlooked a few other points. For example, both
    versions hang onto their buffers even if they'll never be called
    again, but the second could be modified to release it when called
    with a NULL argument, say, if the program can figure out that
    it won't be needed again.

    > I wonder which method is really best. I suppose it depends on how big
    > MAX_STRING_LENGTH is. I assume for instance one wouldn't want to
    > casually declare multi-hundred-kilobyte static arrays?


    "Best" depends on your purposes, and on the constraints and
    opportunities presented by the implementation. That means there's
    no universal answer; there is no best way to define "best." The
    practice of engineering is all about reconciling competing goals;
    it is unlikely the same answer will be chosen every time.

    That said, practical considerations suggest that really large
    static allocations are dubious. They were the life and breath of
    large FORTRAN programs, but C is not FORTRAN.

    --
    Eric Sosman
    lid
     
    Eric Sosman, Jul 27, 2006
    #7
  8. Snis Pilbor

    Ian Collins Guest

    Snis Pilbor wrote:
    > Hi,
    >
    > It seems pretty common to return pointers to a static array, for
    > example:
    >
    > char *capitalize( char *name )
    > {
    > static char buf[MAX_STRING_LENGTH];
    >
    > sprintf( buf, "%s", name );
    > if ( *buf >= 'a' && *buf <= 'z' )
    > *buf += 'A' - 'a';
    >
    > return buf;
    > }
    >
    > But when I do something like this, I like to instead keep a static
    > pointer and malloc it the first time, to save space in whatever bank
    > static memory is stored in, since I have this vague idea that if too
    > much space is used in static memory bad things will happen? So I do
    > this instead:
    >
    > char *capitalize( char *name )
    > {
    > static char *buf;
    >
    > if ( !buf )
    > {
    > buf = malloc( MAX_STRING_LENGTH );
    > if ( !buf ) return name;
    > }
    >
    > sprintf( buf, "%s", name );
    > if ( *buf >= 'a' && *buf <= 'z' )
    > *buf += 'A' - 'a';
    >
    > return buf;
    > }
    >
    > Here is how I analyze both methods as far as efficiency:
    >

    How about a third option:

    char* capitalise( const char* name, char* buff, size_t buffLen );

    Then you can choose whether to pass a dynamic or static buffer. This is
    also one way to make this thread safe.


    --
    Ian Collins.
     
    Ian Collins, Jul 27, 2006
    #8
  9. Snis Pilbor

    John L Guest

    "Snis Pilbor" <> wrote in message news:...
    >
    > EventHelix.com wrote:
    > > Snis Pilbor wrote:
    > > > Hi,
    > > >
    > > > It seems pretty common to return pointers to a static array, for
    > > > example:
    > > >
    > > > char *capitalize( char *name )
    > > > {
    > > > static char buf[MAX_STRING_LENGTH];
    > > >
    > > > sprintf( buf, "%s", name );
    > > > if ( *buf >= 'a' && *buf <= 'z' )
    > > > *buf += 'A' - 'a';
    > > >
    > > > return buf;
    > > > }

    > >
    > > IMHO, Use of static variables inside functions is not a good idea. You
    > > will end up with code that is not thread safe. The function will
    > > misbehave if multiple concurrently running threads are invoking the
    > > function with static variables.
    > >

    >
    > Yes, of course, it goes without saying in either case one must exercise
    > caution.
    > IMHO one of the most beautiful things about C is that it allows you
    > such flexibility. It does not assume you are a baby and hold your
    > hand.
    > If I wanted a language where various people told me "Oh you can't do
    > that, it's too dangerous", I would program Java =) Thanks for the word
    > of warning anyway, though.
    > I will continue to return pointers to static arrays in time sensitive
    > functions because I am, and those who work under me are, smart enough
    > to know what we're doing. =)
    >


    And also smart enough to remember that MAX_STRING_LENGTH does
    not really mean "maximum string length" because the string needs to be
    terminated, and smart enough to remember to fix (or at least check)
    the code which changes letters to upper-case when it needs to cope with
    languages (and alphabets) other than English, and smart enough to ensure
    that the Human Resources Department never recruits anyone less smart
    than you! Good luck.

    --
    John.
     
    John L, Jul 27, 2006
    #9
  10. In article <>,
    Snis Pilbor <> wrote:

    > static char buf[MAX_STRING_LENGTH];


    ....

    > if ( !buf )
    > {
    > buf = malloc( MAX_STRING_LENGTH );
    > if ( !buf ) return name;
    > }


    >* the 2nd version uses less space in delicate "static memory" (not sure
    >what that's called)


    Why is static memory "delicate"?

    >* the 1st is much faster on the first call


    I cannot imagine any circumstance where the time taken by a single
    malloc is significant.

    >* the 1st is slightly faster the other calls (assuming the first time,
    >malloc worked)


    The time for the test is bound to be negligible compared with thet
    sprintf.

    >* the 1st does not assume the OS will do appropriate cleanup on exit,
    >as the 2nd does


    It assumes it will clear up static memory.

    >* the 1st is quicker to type


    How many times are you going to write it?

    >I wonder which method is really best. I suppose it depends on how big
    >MAX_STRING_LENGTH is. I assume for instance one wouldn't want to
    >casually declare multi-hundred-kilobyte static arrays?


    This is the most significant difference: if the function is not called
    at all, the malloc version won't allocate any memory. But even this
    may well be irrelevant on modern operating systems where zero-filled
    memory is not allocated until it is used.

    -- Richard
     
    Richard Tobin, Jul 27, 2006
    #10
  11. In article <>,
    Dave Dunfield <> wrote:

    >- In this particular example, a shortage of memory will cause the program to
    > begin to behave erraticaly {capitalize() stops working}. Better to either
    > have the program fail to load if there is insufficient memory, or provide
    > better fault handling.


    Unless you don't use malloc() (or something that calls it, perhaps
    printf()) anywhere else, this may well just change where the run-time
    failure occurs.

    -- Richard
     
    Richard Tobin, Jul 27, 2006
    #11
  12. >- In this particular example, a shortage of memory will cause the program to
    >> begin to behave erraticaly {capitalize() stops working}. Better to either
    >> have the program fail to load if there is insufficient memory, or provide
    >> better fault handling.


    >Unless you don't use malloc() (or something that calls it, perhaps
    >printf()) anywhere else, this may well just change where the run-time
    >failure occurs.


    Assuming you do not handle potential failures of malloc
    correctly in these "anywhere else" locations....

    I cannot comment on the possible other uses of malloc, only
    the case shown in the example,, which causes an undetected
    change in operation if malloc fails.

    Cheers.
    Dave

    --
    Dunfield Development Services http://www.dunfield.com
    Low cost software development tools for embedded systems
    Software/firmware development services Fax:613-256-5821
     
    Dave Dunfield, Jul 27, 2006
    #12
  13. uninitialized static memory is, in most C implementations, not emitted
    as a long array of zeroes, but just as a note to the linker or C
    sraretup code to add xxx bytes to the zero-initialized "BSS" segment.
    So these variables do not make the executable file any longer.

    Agin, in most C implementations, the BSS area is just taken off the top
    of the free memory pool and doesnt impact on the size of anything
    else-- EXCEPT if you're using an old 16-bit compiler, generating 16-bit
    segments-- there you probably will be limited to 65536 bytes of total
    static data.

    I'd be more concerned about the sprintf() overflowing the array! That
    could be really baad!
     
    Ancient_Hacker, Jul 27, 2006
    #13
  14. Ancient_Hacker said:

    >
    > uninitialized static memory is, in most C implementations,


    Translation: most of those you've encountered.

    --
    Richard Heathfield
    "Usenet is a strange place" - dmr 29/7/1999
    http://www.cpax.org.uk
    email: rjh at above domain (but drop the www, obviously)
     
    Richard Heathfield, Jul 27, 2006
    #14
  15. On Wed, 26 Jul 2006 19:22:43 -0700, EventHelix.com wrote:

    >
    > Snis Pilbor wrote:
    >> Hi,
    >>
    >> It seems pretty common to return pointers to a static array, for
    >> example:
    >>
    >> char *capitalize( char *name )
    >> {
    >> static char buf[MAX_STRING_LENGTH];
    >>
    >> sprintf( buf, "%s", name );
    >> if ( *buf >= 'a' && *buf <= 'z' )
    >> *buf += 'A' - 'a';
    >>
    >> return buf;
    >> }

    >
    > IMHO, Use of static variables inside functions is not a good idea. You
    > will end up with code that is not thread safe. The function will
    > misbehave if multiple concurrently running threads are invoking the
    > function with static variables.

    And worse, to my mind, is that with either version
    printf( "%s %s\n", capitalise( "bob"), capitalise( "mary"));
    will be disappointing.
     
    Duncan Muirhead, Jul 27, 2006
    #15
  16. Richard Heathfield wrote:
    > Ancient_Hacker said:
    >
    > >
    > > uninitialized static memory is, in most C implementations,

    >
    > Translation: most of those you've encountered.



    Okay, how's about you compile this little program and tell us the size
    of the obj and executable files:

    int a[1000000];

    int main( int argc, char * argv[] ) {

    a[1] = 99;
    printf("%d\n", A[1] );
    return 0;
    }

    I would LOVE to learn what C compilers are so poorly conceived as to
    emit uninitialized static data as initialized data.

    I've probably had the acquaintance of a dozen C compilers over the
    centuries, and I don't recall a one that was quite this clunky. I'd
    love to learn of counterexamples.
     
    Ancient_Hacker, Jul 27, 2006
    #16
  17. Snis Pilbor

    Chris Dollin Guest

    Ancient_Hacker wrote:

    >
    > Richard Heathfield wrote:
    >> Ancient_Hacker said:
    >>
    >> >
    >> > uninitialized static memory is, in most C implementations,

    >>
    >> Translation: most of those you've encountered.

    >
    >
    > Okay, how's about you compile this little program and tell us the size
    > of the obj and executable files:
    >
    > int a[1000000];
    >
    > int main( int argc, char * argv[] ) {
    >
    > a[1] = 99;
    > printf("%d\n", A[1] );
    > return 0;
    > }
    >
    > I would LOVE to learn what C compilers are so poorly conceived as to
    > emit uninitialized static data as initialized data.


    I /think/ early versions of Norcroft C under RISC OS did that, but my
    memory may be misfiring.

    --
    Chris "nortalgic" Dollin
    "People are part of the design. It's dangerous to forget that." /Star Cops/
     
    Chris Dollin, Jul 27, 2006
    #17
  18. Snis Pilbor posted:

    > * the 1st does not assume the OS will do appropriate cleanup on exit,
    > as the 2nd does



    void *p; /* Global variable */

    void DelDynam(void)
    {
    free(p);
    }


    Then inside you function:

    if ( !p )
    {
    p = malloc( MAX_STRING_LENGTH );
    if ( !p ) return name;

    atexit(DelDynam);
    }

    --

    Frederick Gotham
     
    Frederick Gotham, Jul 27, 2006
    #18
  19. Ancient_Hacker said:

    >
    > Richard Heathfield wrote:
    >> Ancient_Hacker said:
    >>
    >> >
    >> > uninitialized static memory is, in most C implementations,

    >>
    >> Translation: most of those you've encountered.

    >
    >
    > Okay, how's about you compile this little program and tell us the size
    > of the obj and executable files:
    >
    > int a[1000000];


    Stop right there. No implementation is obliged to support an object that
    size.

    <snip>

    --
    Richard Heathfield
    "Usenet is a strange place" - dmr 29/7/1999
    http://www.cpax.org.uk
    email: rjh at above domain (but drop the www, obviously)
     
    Richard Heathfield, Jul 27, 2006
    #19
  20. Snis Pilbor

    Bas Wassink Guest

    On Thu, 27 Jul 2006 13:46:21 +0000, Frederick Gotham wrote:

    > Snis Pilbor posted:
    >
    >> * the 1st does not assume the OS will do appropriate cleanup on exit,
    >> as the 2nd does

    >
    >
    > void *p; /* Global variable */
    >
    > void DelDynam(void)
    > {
    > free(p);
    > }
    >
    >
    > Then inside you function:
    >
    > if ( !p )
    > {
    > p = malloc( MAX_STRING_LENGTH );
    > if ( !p ) return name;
    >
    > atexit(DelDynam);
    > }


    I don't want to sound like a nitpick, but this will fail if atexit() fails
    (it returns 0 on success and non-0 on failure), so you might want inform
    the user if this happens.
    I personally don't bother with atexit() and do all cleanup myself.
     
    Bas Wassink, Jul 27, 2006
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. sikka noel
    Replies:
    8
    Views:
    422
    Mike Wahler
    Aug 5, 2003
  2. Lonnie Princehouse
    Replies:
    0
    Views:
    292
    Lonnie Princehouse
    Apr 14, 2004
  3. vlsidesign
    Replies:
    26
    Views:
    987
    Keith Thompson
    Jan 2, 2007
  4. Cliff  Martin
    Replies:
    1
    Views:
    3,042
    Larry Smith
    Jan 31, 2007
  5. SM
    Replies:
    9
    Views:
    507
Loading...

Share This Page