bad programming practice?

Discussion in 'C Programming' started by fabio, Apr 18, 2006.

  1. fabio

    fabio Guest

    it's bad to make a thing like:

    //declaration of variables
    //some code...

    n = inputdata

    int array[n];


    in other words, to define an array after the retreival (via input data
    o after some calculations) of his dimension?
    (to avoid make a list)

    i think no, but i ask :)
    fabio, Apr 18, 2006
    #1
    1. Advertising

  2. fabio wrote:
    > it's bad to make a thing like:
    >
    > //declaration of variables
    > //some code...
    >
    > n = inputdata
    >
    > int array[n];
    >
    >
    > in other words, to define an array after the retreival (via input data
    > o after some calculations) of his dimension?
    > (to avoid make a list)


    You don't have to make a list. Declare a pointer to the first element:

    int *array;

    Then `malloc()` as much as you need.

    array = malloc( n * sizeof *array);

    You can then use

    array[index]

    syntax to get to your elements. Do check `malloc()` succeeded.
    Vladimir S. Oka, Apr 18, 2006
    #2
    1. Advertising

  3. fabio

    Richard Bos Guest

    fabio <> wrote:

    > n = inputdata
    >
    > int array[n];
    >
    > in other words, to define an array after the retreival (via input data
    > o after some calculations) of his dimension?


    That is legal under the 1999 ISO C Standard, but not under the 1989
    Standard. You compiler a. probably is a C89 compiler and b. possibly
    allows this as an extension anyway.

    If you want to avoid it for reasons of portability, you can always
    resort to malloc().

    Richard
    Richard Bos, Apr 18, 2006
    #3
  4. fabio

    Guest

    Vladimir S. Oka wrote:
    > fabio wrote:
    > > it's bad to make a thing like:
    > >
    > > //declaration of variables
    > > //some code...
    > >
    > > n = inputdata
    > >
    > > int array[n];
    > >
    > >
    > > in other words, to define an array after the retreival (via input data
    > > o after some calculations) of his dimension?
    > > (to avoid make a list)

    >
    > You don't have to make a list. Declare a pointer to the first element:
    >
    > int *array;
    >
    > Then `malloc()` as much as you need.
    >
    > array = malloc( n * sizeof *array);

    Don't do it like this, this is a very very bad idom for malloc()'ing n
    objects for an array
    if at all possible use calloc() i.e. calloc(n, sizeof(array))
    since calloc() doe size checking for you, if can't use calloc() (wich
    would be strange)
    check it by hand.

    The thing is that you might overflow your type's here,
    e.g.
    size_t a, b;
    char *ptr;
    if (( ptr = malloc(a, b)) == NULL) /* overflow */
    return (NULL);
    Will overflow is a or b == ULONG_MAX (wrap around)
    -> Only "one" example, anyone who can handle basic artithmetic will see
    the point.

    You might accidently allocated more resources then you think you have
    and well,
    buffer overflow ?

    >
    > You can then use
    >
    > array[index]
    >
    > syntax to get to your elements. Do check `malloc()` succeeded.
    , Apr 18, 2006
    #4
  5. In article <>,
    <> wrote:
    >Vladimir S. Oka wrote:


    >> array = malloc( n * sizeof *array);


    >Don't do it like this, this is a very very bad idom for malloc()'ing n
    >objects for an array


    I don't think I've ever seen anyone say that it is a bad idiom,
    let alone a "very very bad idiom".


    >if at all possible use calloc() i.e. calloc(n, sizeof(array))
    >since calloc() doe size checking for you,


    C89 4.10.3.1 The Calloc Function

    Description
    The calloc function allocates space for an array of nmemb objects,
    each of whose size is size. The space is initialized to all bits zero.

    Returns
    The calloc function returns either a null pointer or a pointer to
    the allocated space.


    I don't see anything in there about calloc() checking sizes.

    What I do see is that calloc() always initializes to zero, which malloc()
    does not do. If one is working with a large array, the time required to
    perform that initialization could be high, and it might be entirely
    wasted time if the user program promptly sets the storage to something else.

    On systems in which the zeroing is done in software, calloc() would
    cause the entire array to pass through the cache. If calloc() is
    written to optimize the time required to do the initialization, that
    would have the side effect of pushing all other data (and possibly most
    code as well) out of the cache. That could have substantial hidden
    costs, especially on multiprocessor shared memory systems in which it
    is necessary to communicate the memory addresses to all nodes in order
    to "lock" the memory segment while it is written.

    On systems in which the zeroing can be done in hardware ("demand zero
    memory"), memory which is being allocated out of the existing pool must
    still be zeroed in software, whereas memory that happens to get allocated
    by a system call asking for more memory would not [on such systems]
    require explicit initialization. The timing difference is detectable
    for cryptographic differential analysis purposes; there are a lot
    of other timing differences that can be detected for other operations,
    but if one is trying to protect against such issues, the fewer issues
    that one has to worry about, the better.


    In summary, using malloc() instead of calloc() is NOT a
    "very very bad idiom": there are solid reasons to prefer malloc()
    in a number of instances.
    --
    Okay, buzzwords only. Two syllables, tops. -- Laurie Anderson
    Walter Roberson, Apr 18, 2006
    #5
  6. In article <e2317k$aik$>,
    Walter Roberson <-cnrc.gc.ca> wrote:

    >I don't see anything in there about calloc() checking sizes.


    I think he meant that calloc() calculates the space needed for n
    objects of given size, whereas with malloc() you have to calculate it
    yourself. Of course it's not much harder to type an asterisk than a
    comma, but it's theoretically true that calloc() may deal better with
    overflow. (On the other hand, on most systems if the multiplication
    overflows then it won't be able to allocate the memory anyway.)

    --Richard
    Richard Tobin, Apr 18, 2006
    #6
  7. wrote:
    > Vladimir S. Oka wrote:
    > >
    > > array = malloc( n * sizeof *array);

    >
    > Don't do it like this, this is a very very bad idom for malloc()'ing n
    > objects for an array


    I would very very much like to hear why?

    > if at all possible use calloc() i.e. calloc(n, sizeof(array))
    > since calloc() doe size checking for you, if can't use calloc() (wich
    > would be strange) check it by hand.


    Have a look at Walter's reply...

    > The thing is that you might overflow your type's here,
    > e.g.
    > size_t a, b;
    > char *ptr;
    > if (( ptr = malloc(a, b)) == NULL) /* overflow */


    Since when `malloc()` takes two parameters? You mean `calloc()`?

    > return (NULL);


    If you really wanted this, you could do:

    return ptr = calloc(a, b);

    What's this snippet all about?

    > You might accidently allocated more resources then you think you have
    > and well, buffer overflow ?


    You should have read the OP more carefully. It was about learning in
    advance how many elements you need, then allocating that exact number.
    If there's not enough memory for that, `malloc()` will fail.

    I also can't see where the buffer overflow comes in.
    Vladimir S. Oka, Apr 18, 2006
    #7
  8. fabio

    tedu Guest

    Richard Tobin wrote:
    > In article <e2317k$aik$>,
    > Walter Roberson <-cnrc.gc.ca> wrote:
    >
    > >I don't see anything in there about calloc() checking sizes.


    calloc allocates enough space for n objects of size s. if n * s
    overflows, then it has to return NULL. returning a non-NULL pointer
    means your library is broken, since the allocated memory is not big
    enough for n objects of size s.

    > overflow. (On the other hand, on most systems if the multiplication
    > overflows then it won't be able to allocate the memory anyway.)


    if the multiplication overflows, it has a decent chance of being a
    small number. things go downhill when you start treating your very
    small allocation as a very big allocation.
    tedu, Apr 18, 2006
    #8
  9. fabio

    tedu Guest

    Vladimir S. Oka wrote:

    > > You might accidently allocated more resources then you think you have
    > > and well, buffer overflow ?

    >
    > You should have read the OP more carefully. It was about learning in
    > advance how many elements you need, then allocating that exact number.
    > If there's not enough memory for that, `malloc()` will fail.
    >
    > I also can't see where the buffer overflow comes in.


    #include <stdint.h>
    #include <stdio.h>
    #include <stdlib.h>

    int main() {
    size_t s, n;
    void *p, *p2;

    s = 12;
    n = (SIZE_MAX / s) + 1;

    p = malloc(s * n);
    p2 = calloc(s, n);

    printf("%lu %lu %lu %p %p\n", s, n, n * s, p, p2);
    }

    on a fairly common 32 bit machine gives me:
    12 357913942 8 0x81af008 (nil)

    dereferencing p is probably going to be a mistake, seeing how it
    doesn't have enough space for even one of my 12 byte elements. malloc
    did not fail, and there is clearly not enough space.
    tedu, Apr 18, 2006
    #9
  10. fabio

    Chris Smith Guest

    Richard Tobin <> wrote:
    > I think he meant that calloc() calculates the space needed for n
    > objects of given size, whereas with malloc() you have to calculate it
    > yourself. Of course it's not much harder to type an asterisk than a
    > comma, but it's theoretically true that calloc() may deal better with
    > overflow. (On the other hand, on most systems if the multiplication
    > overflows then it won't be able to allocate the memory anyway.)


    Of course, the problem is that the multiplication may overflow to
    produce a small positive integer, which would then cause the allocation
    to "succeed" (FSVO succeed). The ensuing reads and writes beyond the
    end of allocated space would be dangerous, and would crash the program
    if you're lucky.

    I won't claim to know if calloc is subject to the same problems, but
    your statement above about calloc handling overflow better suggests that
    it is not.

    --
    www.designacourse.com
    The Easiest Way To Train Anyone... Anywhere.

    Chris Smith - Lead Software Developer/Technical Trainer
    MindIQ Corporation
    Chris Smith, Apr 18, 2006
    #10
  11. fabio

    Guest

    Yes, it's a very very very very bad programming style. Because it's
    plain old. It opens up your code to what is called a stack overflow.
    You read n from the input and than the array is allocated on the stack
    and oops: a mallicios user enters n = 100.000.000 and unless your
    system handles a 400MB stack, you've got yourself a problem. Good luck
    with
    it!

    Code:
    stefan@localhost ~/tests $ cat stackoverflow.c
    #include <stdio.h>
    
    int main()
    {
            int n;
            scanf("%d", &n);
            int a[n];
            int i;
            for (i = 0; i < n; ++i)
            {
                    a[i] = i * i + i;
            }
            printf("%d", a[n - 1]);
    
            return 0;
    }
    stefan@localhost ~/tests $ gcc -Wall -o stackoverflow stackoverflow.c
    stefan@localhost ~/tests $ ./stackoverflow
    10000
    99990000stefan@localhost ~/tests $ ./stackoverflow
    100000000
    Segmentation fault
    
    , Apr 18, 2006
    #11
  12. fabio

    Old Wolf Guest

    tedu wrote:
    > s = 12;
    > n = (SIZE_MAX / s) + 1;
    >
    > p = malloc(s * n);
    > p2 = calloc(s, n);


    Trying to allocate more than SIZE_MAX bytes causes undefined
    behaviour. Don't do it. If you think this might be a problem, then
    check the sizes of 's' and 'n' before calling ?alloc. You can't
    rely on calloc doing something sensible in the case that you try
    and allocate too much.
    Old Wolf, Apr 18, 2006
    #12
  13. fabio

    jacob navia Guest

    Old Wolf a écrit :
    > tedu wrote:
    >
    >> s = 12;
    >> n = (SIZE_MAX / s) + 1;
    >>
    >> p = malloc(s * n);
    >> p2 = calloc(s, n);

    >
    >
    > Trying to allocate more than SIZE_MAX bytes causes undefined
    > behaviour. Don't do it. If you think this might be a problem, then
    > check the sizes of 's' and 'n' before calling ?alloc. You can't
    > rely on calloc doing something sensible in the case that you try
    > and allocate too much.
    >


    You can't rely on calloc returning NULL ?????????

    That would be news to me sorry.

    "The calloc function returns either a null pointer or a pointer to the
    allocated space."

    C standard page 312
    jacob navia, Apr 18, 2006
    #13
  14. fabio

    Al Balmer Guest

    On 18 Apr 2006 15:08:02 -0700, wrote:

    >Yes, it's a very very very very bad programming style. Because it's
    >plain old. It opens up your code to what is called a stack overflow.
    >You read n from the input and than the array is allocated on the stack
    >and oops: a mallicios user enters n = 100.000.000 and unless your
    >system handles a 400MB stack, you've got yourself a problem. Good luck
    >with
    >it!
    >

    Has it occurred to you that it's possible to check the value of n
    before doing the allocation?

    >
    Code:
    >stefan@localhost ~/tests $ cat stackoverflow.c
    >#include <stdio.h>
    >
    >int main()
    >{
    >        int n;
    >        scanf("%d", &n);
    >        int a[n];
    >        int i;
    >        for (i = 0; i < n; ++i)
    >        {
    >                a[i] = i * i + i;
    >        }
    >        printf("%d", a[n - 1]);
    >
    >        return 0;
    >}
    >stefan@localhost ~/tests $ gcc -Wall -o stackoverflow stackoverflow.c
    >stefan@localhost ~/tests $ ./stackoverflow
    >10000
    >99990000stefan@localhost ~/tests $ ./stackoverflow
    >100000000
    >Segmentation fault
    >


    --
    Al Balmer
    Sun City, AZ
    Al Balmer, Apr 19, 2006
    #14
  15. Al Balmer <> writes:
    > On 18 Apr 2006 15:08:02 -0700, wrote:
    >>Yes, it's a very very very very bad programming style. Because it's
    >>plain old. It opens up your code to what is called a stack overflow.
    >>You read n from the input and than the array is allocated on the stack
    >>and oops: a mallicios user enters n = 100.000.000 and unless your
    >>system handles a 400MB stack, you've got yourself a problem. Good luck
    >>with
    >>it!
    >>

    > Has it occurred to you that it's possible to check the value of n
    > before doing the allocation?


    Yes, but check it against what?

    At one time, it would have been reasonable to assume (on most systems)
    that a request to allocate a megabyte of memory must be an error. A
    program you write today might be used 10 years from now, and might
    reasonably allocate many gigabytes. It's impossible to know what a
    reasonable limit might be without knowing more about what the program
    is doing.

    Variable-length arrays (which, if it hasn't already been mentioned in
    this thread, are supported only in C99 <OT>and by gcc, and perhaps
    other compilers, as an extension</OT>) have a major drawback: if you
    declare one that's too big to fit in memory, you get undefined
    behavior. (The same is true for fixed-size arrays, but at least it's
    easier to choose the size when you write the program.)

    If you give malloc() or calloc() an unreasonably large size, such as
    SIZE_MAX, it will fail cleanly and give you a null pointer, which you
    can then handle as you choose.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    We must do something. This is something. Therefore, we must do this.
    Keith Thompson, Apr 19, 2006
    #15
  16. tedu opined:

    > Vladimir S. Oka wrote:
    >
    >> > You might accidently allocated more resources then you think you
    >> > have and well, buffer overflow ?

    >>
    >> You should have read the OP more carefully. It was about learning in
    >> advance how many elements you need, then allocating that exact
    >> number. If there's not enough memory for that, `malloc()` will fail.
    >>
    >> I also can't see where the buffer overflow comes in.

    >
    > #include <stdint.h>
    > #include <stdio.h>
    > #include <stdlib.h>
    >
    > int main() {
    > size_t s, n;
    > void *p, *p2;
    >
    > s = 12;
    > n = (SIZE_MAX / s) + 1;
    >
    > p = malloc(s * n);
    > p2 = calloc(s, n);
    >
    > printf("%lu %lu %lu %p %p\n", s, n, n * s, p, p2);
    > }
    >
    > on a fairly common 32 bit machine gives me:
    > 12 357913942 8 0x81af008 (nil)
    >
    > dereferencing p is probably going to be a mistake, seeing how it
    > doesn't have enough space for even one of my 12 byte elements.
    > malloc did not fail, and there is clearly not enough space.


    Ah, I see. I'd still rather check for that problem before, and still do
    `malloc()` as especially with huge amounts of memory `calloc()` may
    have a big performance penalty (and would still fail, albeit in a more
    graceful manner).

    --
    "However, complexity is not always the enemy."

    -- Larry Wall (Open Sources, 1999 O'Reilly and Associates)

    <http://clc-wiki.net/wiki/Introduction_to_comp.lang.c>
    Vladimir S. Oka, Apr 19, 2006
    #16
  17. fabio

    Al Balmer Guest

    On Tue, 18 Apr 2006 23:59:50 GMT, Keith Thompson <>
    wrote:

    >Al Balmer <> writes:
    >> On 18 Apr 2006 15:08:02 -0700, wrote:
    >>>Yes, it's a very very very very bad programming style. Because it's
    >>>plain old. It opens up your code to what is called a stack overflow.
    >>>You read n from the input and than the array is allocated on the stack
    >>>and oops: a mallicios user enters n = 100.000.000 and unless your
    >>>system handles a 400MB stack, you've got yourself a problem. Good luck
    >>>with
    >>>it!
    >>>

    >> Has it occurred to you that it's possible to check the value of n
    >> before doing the allocation?

    >
    >Yes, but check it against what?


    Oops, I'm in the wrong thread (the malloc vs. calloc one. (I think the
    "very very very very" fooled me.) And not reading closely enough - I
    should have noticed the word "stack." For this thread, I agree that a
    variable length array is not the way to go. In fact, I'd go further -
    I've survived without variable arrays til now, and can't think of
    anything I couldn't do without them.
    >


    --
    Al Balmer
    Sun City, AZ
    Al Balmer, Apr 19, 2006
    #17
    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. Stanley
    Replies:
    0
    Views:
    371
    Stanley
    Aug 29, 2003
  2. Kevin Spencer

    Re: Bad practice? Using #Include in ASP.NET

    Kevin Spencer, Aug 29, 2003, in forum: ASP .Net
    Replies:
    0
    Views:
    437
    Kevin Spencer
    Aug 29, 2003
  3. xyZed
    Replies:
    2
    Views:
    386
    David Dorward
    Jan 24, 2006
  4. jon wayne
    Replies:
    1
    Views:
    372
    benben
    Sep 19, 2005
  5. rantingrick
    Replies:
    44
    Views:
    1,170
    Peter Pearson
    Jul 13, 2010
Loading...

Share This Page