Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > bad programming practice?

Reply
Thread Tools

bad programming practice?

 
 
stefan.ciobaca@gmail.com
Guest
Posts: n/a
 
      04-18-2006
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
 
Reply With Quote
 
 
 
 
Old Wolf
Guest
Posts: n/a
 
      04-18-2006
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.

 
Reply With Quote
 
 
 
 
jacob navia
Guest
Posts: n/a
 
      04-18-2006
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

 
Reply With Quote
 
Al Balmer
Guest
Posts: n/a
 
      04-18-2006
On 18 Apr 2006 15:08:02 -0700, http://www.velocityreviews.com/forums/(E-Mail Removed) 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
 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      04-18-2006
Al Balmer <(E-Mail Removed)> writes:
> On 18 Apr 2006 15:08:02 -0700, (E-Mail Removed) 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) (E-Mail Removed) <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.
 
Reply With Quote
 
Vladimir S. Oka
Guest
Posts: n/a
 
      04-19-2006
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>

 
Reply With Quote
 
Al Balmer
Guest
Posts: n/a
 
      04-19-2006
On Tue, 18 Apr 2006 23:59:50 GMT, Keith Thompson <(E-Mail Removed)>
wrote:

>Al Balmer <(E-Mail Removed)> writes:
>> On 18 Apr 2006 15:08:02 -0700, (E-Mail Removed) 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
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
integer >= 1 == True and integer.0 == False is bad, bad, bad!!! rantingrick Python 44 07-13-2010 06:33 PM
Bad media, bad files or bad Nero? John Computer Information 23 01-08-2008 09:17 PM
ActiveX apologetic Larry Seltzer... "Sun paid for malicious ActiveX code, and Firefox is bad, bad bad baad. please use ActiveX, it's secure and nice!" (ok, the last part is irony on my part) fernando.cassia@gmail.com Java 0 04-16-2005 10:05 PM
24 Season 3 Bad Bad Bad (Spoiler) nospam@nospam.com DVD Video 12 02-23-2005 03:28 AM
24 Season 3 Bad Bad Bad (Spoiler) nospam@nospam.com DVD Video 0 02-19-2005 01:10 AM



Advertisments