bad programming practice?

[fabio]

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

 

[Vladimir S. Oka]

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.

 

[Richard Bos]

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

 

[bzthib]

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.

 

[Walter Roberson]

Vladimir S. Oka wrote:

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.

 

[Richard Tobin]

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

 

[Vladimir S. Oka]

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.

 

[tedu]

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]

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.

 

[Chris Smith]

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

 

[stefan.ciobaca]

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!

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

 

[Old Wolf]

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.

 

[jacob navia]

Old Wolf a écrit :

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

 

[Al Balmer]

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?

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

 

[Keith Thompson]

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.

 

[Vladimir S. Oka]

tedu opined:

#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>

 

[Al Balmer]

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.