Velocity Reviews > MALLOC problem

# MALLOC problem

Guest
Posts: n/a

 10-15-2010
Hello friends

I do not got Malloc to work

look at this source code and tell me whats wrong. I have marked the 2
problem areas.

/* This program will sort an array of floats from the user.*/

#include <stdio.h>
#include <stdlib.h>
/* prints the contents of the array to stdout. array is the array
to print, size is it's size */
void prn_array(float *array, int size) {
int loopcount; /* used as a loop counter */
printf("\nThe sorted array is:\n");
for (loopcount=0; loopcount < size; loopcount++)
printf("%d ",array[loopcount]);
printf("\n");
}

/* swaps two floats from the array. num_one and num_two are the floats */
int swap( float *num_one, float *num_two) {

float temporary; /* temporary variable */
temporary = *num_one;
*num_one = *num_two;
*num_two = temporary;

}

/* sorts array in descending order. array is the array
to print, size is it's size */
void sort_descending (float array[], int size) {

int i, j; /* integers */

for (i=0; i<size - 1; ++i)
for (j= size-1; i<j;--j)
if (array[j-1] < array[j])
swap(&array[j-1], &array[j]);

}

/* sorts array in ascending order. array is the array
to print, size is it's size */
void sort_ascending (float array[], int size) {

int i, j; /* integers */

for (i=size; i>0 - 1; --i)
for (j= 0-1; i>j;++j)
if (array[j-1] > array[j])
swap(&array[j-1], &array[j]);

}

main() {

int size; /* number of floats in the array */
int *pointer, loopcount, choice; /* pointer is an int pointer,
loopcount is used as a loop counter, choice denotes users choice */
float array[size]; /* <---------------------- Problem one */

/* A welcome message to the user*/
printf("\nWelcome in this program
you will enter an array of floats and the program will sort it in
ascending
or descending order and and finally print it");
/* asking for the numbers of floats in the array */
printf("\nHow many numbers would you like sorted? ");
/* read user input and place into size variable */
scanf ("%d, &size");

array= (float *)malloc(size * sizeof(float)); /* <----------- Problem
two */

/* asking for the array */
/* branch based on size entered */
if(size==1)
else

for (loopcount=0;loopcount<size; loopcount++)
scanf("%f", array[loopcount]);
while (choice==0){
printf("\n Please choose one of the following:");
printf("\n 1. Sort in ascending order ");
printf("\n 2. Sort in descending order");
printf("\n Enter choice --> ");
scanf ("%d", choice);
if ((choice==1)||(choice==2))
break;
}
/* which sort to use, branch on choice */
if(choice==1)
sort_ascending(array, size); /* sort the array in descending order */
else if (choice==2)
sort_descending(array, size); /* sort the array in descending order
*/

/* output the contents of the array */
prn_array(array, size);

}

Seebs
Guest
Posts: n/a

 10-15-2010
On 2010-10-15, sajjad <(E-Mail Removed)> wrote:
> I do not got Malloc to work

Okay.

You know... Your subject line says "MALLOC". Here, you say "Malloc".

C is case sensitive. I urge you to put in the extra effort to be
consistent and correct with capitalization even when writing about C,
you'll find that it makes it easier to get programs right.

> look at this source code and tell me whats wrong. I have marked the 2
> problem areas.

No, you haven't.

> /* prints the contents of the array to stdout. array is the array
> to print, size is it's size */
> void prn_array(float *array, int size) {
> int loopcount; /* used as a loop counter */
> printf("\nThe sorted array is:\n");
> for (loopcount=0; loopcount < size; loopcount++)
> printf("%d ",array[loopcount]);

You did not mark this, but it is in fact a problem area, because you
are passing a float to printf, but giving it a %d format specifier
which expects an int. Try
printf("%f ", array[loopcount]);

> /* sorts array in descending order. array is the array
> to print, size is it's size */
> void sort_descending (float array[], int size) {
> int i, j; /* integers */

This comment is completely useless. Don't put in a comment if
it isn't going to give the reader any information.

> for (i=0; i<size - 1; ++i)
> for (j= size-1; i<j;--j)

Your spacing here is inconsistent. While this has no immediate effect
on generated code, it makes life harder for the reader. Try to be
consistent about spacing. For instance, in these two lines, you have
no spaces on either side of the '<', but on the next line, you
have spaces on both sides.

1. It makes the code harder to read.
2. It makes the reader distrust you.

> if (array[j-1] < array[j])
> swap(&array[j-1], &array[j]);

This is pretty inefficient.

> main() {

You really ought to declare this properly as "int main(void)".

> int size; /* number of floats in the array */

This is a good comment.

> int *pointer, loopcount, choice; /* pointer is an int pointer,
> loopcount is used as a loop counter, choice denotes users choice */

Again, the comment "is an int pointer" is TOTALLY USELESS. Don't give

> float array[size]; /* <---------------------- Problem one */

Okay, here's your problem. What is "size" *right now*? Well, you haven't
initialized it, so it's garbage. You've just tried to declare an array
of unknown size.

In C89, you can't do this at all -- array sizes have to be constant.
In C99, you could do this if you had already set "size" to the right
value. But it's almost certainly the case that you don't mean to do
this at all, since you're talking about "malloc". So try:
float *array;

This will let you declare a pointer which can point to the storage you

> /* read user input and place into size variable */
> scanf ("%d, &size");

Okay.

> array= (float *)malloc(size * sizeof(float)); /* <----------- Problem
> two */

First off, drop the cast -- it's useless in C. Secondly, you're trying
to assign to an array. Arrays are not modifiable. You can modify their
elements, but not the array itself -- it's not a modifiable lvalue.

If you had declared a POINTER, rather than an array, this could work.

(If you are about to say something about how pointers and arrays are
the same thing, stop and go read the comp.lang.c FAQ section on pointers
and arrays.)

> for (loopcount=0;loopcount<size; loopcount++)
> scanf("%f", array[loopcount]);

Here, you have omitted the "&" you need. You can't pass the float to
scanf; you have to pass its ADDRESS to scanf, just as you did above when
you used scanf for size.

> while (choice==0){
> printf("\n Please choose one of the following:");
> printf("\n 1. Sort in ascending order ");
> printf("\n 2. Sort in descending order");
> printf("\n Enter choice --> ");
> scanf ("%d", choice);
> if ((choice==1)||(choice==2))
> break;
> }

Consider what will happen if the user makes a mistake and enters "3" here.
Your loop terminates, but choice will not be 1 or 2.

> /* which sort to use, branch on choice */
> if(choice==1)
> sort_ascending(array, size); /* sort the array in descending order */
> else if (choice==2)
> sort_descending(array, size); /* sort the array in descending order
> */

These comments, again, are pretty useless.

See above. The very shortest path to a fix will be the %d=>%f fix,
adding the & on array[loopcount], and fixing the declaration of array.
However, there's many other things you should improve.

Remember this: Sloppy work is habit-forming. Take every program you
write seriously, respect it, and do your best every time. If you do this,
you will find programming a great deal more fun than if you try to cut
corners on programs you don't think are important.

-s
--
Copyright 2010, all wrongs reversed. Peter Seebach / http://www.velocityreviews.com/forums/(E-Mail Removed)
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
I am not speaking for my employer, although they do rent some of my opinions.

Morris Keesan
Guest
Posts: n/a

 10-16-2010
On Fri, 15 Oct 2010 17:03:46 -0400, Seebs <(E-Mail Removed)> wrote:

> On 2010-10-15, sajjad <(E-Mail Removed)> wrote:

<big snip>
>> /* read user input and place into size variable */
>> scanf ("%d, &size");

>
> Okay.

Not okay.
--
Morris Keesan -- (E-Mail Removed)

Seebs
Guest
Posts: n/a

 10-16-2010
On 2010-10-16, Morris Keesan <(E-Mail Removed)> wrote:
> On Fri, 15 Oct 2010 17:03:46 -0400, Seebs <(E-Mail Removed)> wrote:
>
>> On 2010-10-15, sajjad <(E-Mail Removed)> wrote:

><big snip>
>>> /* read user input and place into size variable */
>>> scanf ("%d, &size");

>> Okay.

> Not okay.

.... Woah. I can't believe I missed that.

Sajjad: This is why it's important to be good and consistent about your
spacing and layout -- it makes things like this easier to spot.

Basically, it's pretty hard for people to actually process all of the
details at once consciously (perhaps impossible for most of us), so we
rely heavily on our less-conscious mind's ability to pre-sort the
data it presents to us. Style conventions make that task much
easier.

-s
--
Copyright 2010, all wrongs reversed. Peter Seebach / (E-Mail Removed)
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
I am not speaking for my employer, although they do rent some of my opinions.

luser- -droog
Guest
Posts: n/a

 10-16-2010
On Oct 15, 9:44*pm, Seebs <(E-Mail Removed)> wrote:
> On 2010-10-16, Morris Keesan <(E-Mail Removed)> wrote:
>
> > On Fri, 15 Oct 2010 17:03:46 -0400, Seebs <(E-Mail Removed)> wrote:

>
> >> On 2010-10-15, sajjad <(E-Mail Removed)> wrote:

> ><big snip>
> >>> * * /* read user input and place into size variable */
> >>> * * scanf ("%d, &size");
> >> Okay.

> > Not okay.

>
> ... Woah. *I can't believe I missed that.
>
> Sajjad: *This is why it's important to be good and consistent about your
> spacing and layout -- it makes things like this easier to spot.
>
> Basically, it's pretty hard for people to actually process all of the
> details at once consciously (perhaps impossible for most of us), so we
> rely heavily on our less-conscious mind's ability to pre-sort the
> data it presents to us. *Style conventions make that task much
> easier.
>

I'm relatively new at this, but I think you've been trolled.
You could have probably substituted FAQ citations for each
sentence you typed. In fact, he probably printed the FAQ TOC,
made confetti, and grabbed a handful of suggestions to consciously
violate for each line he typed.

I mean really: loopcounter /* the loop counter */
REALLY?

--
sorry for shouting.

Nobody
Guest
Posts: n/a

 10-16-2010
On Fri, 15 Oct 2010 23:19:45 -0700, luser- -droog wrote:

> I mean really: loopcounter /* the loop counter */
> REALLY?

It's not that uncommon. Take a self-taught programmer who has never
bothered to comment their code, and put them on a programming course.
They'll be told how important it is to comment code, so they'll start

Guest
Posts: n/a

 10-16-2010
Nobody writes:

> On Fri, 15 Oct 2010 23:19:45 -0700, luser- -droog wrote:
>
>> I mean really: loopcounter /* the loop counter */ REALLY?

>
> It's not that uncommon. Take a self-taught programmer who has never
> bothered to comment their code, and put them on a programming course.
> They'll be told how important it is to comment code, so they'll start

Our course teaches good programming practise - EVERY variable must be
commented, also EVERY functions purpose explained and EVERY argument
commented, and only use meaningful names for functions and variables.

Ian Collins
Guest
Posts: n/a

 10-16-2010
On 10/16/10 08:52 PM, sajjad wrote:
> Nobody writes:
>
>> On Fri, 15 Oct 2010 23:19:45 -0700, luser- -droog wrote:
>>
>>> I mean really: loopcounter /* the loop counter */ REALLY?

>>
>> It's not that uncommon. Take a self-taught programmer who has never
>> bothered to comment their code, and put them on a programming course.
>> They'll be told how important it is to comment code, so they'll start

>
> Our course teaches good programming practise - EVERY variable must be
> commented, also EVERY functions purpose explained and EVERY argument
> commented, and only use meaningful names for functions and variables.

Pointless comments are a distraction, not good programming practise.

--
Ian Collins

Seebs
Guest
Posts: n/a

 10-16-2010
On 2010-10-16, sajjad <(E-Mail Removed)> wrote:
> Our course teaches good programming practise

No, it doesn't.

> - EVERY variable must be
> commented,

That's not really a particularly good practice.

> also EVERY functions purpose explained and EVERY argument
> commented, and only use meaningful names for functions and variables.

You don't really do any of these, though.

-s
--
Copyright 2010, all wrongs reversed. Peter Seebach / (E-Mail Removed)
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
I am not speaking for my employer, although they do rent some of my opinions.

luser- -droog
Guest
Posts: n/a

 10-16-2010
On Oct 16, 2:22*am, Nobody <(E-Mail Removed)> wrote:
> On Fri, 15 Oct 2010 23:19:45 -0700, luser- -droog wrote:
> > I mean really: loopcounter /* the loop counter */
> > REALLY?

>
> It's not that uncommon. Take a self-taught programmer who has never
> bothered to comment their code, and put them on a programming course.
> They'll be told how important it is to comment code, so they'll start

But it does signal a gap in the hand-eye-brain circuit.
I mean ... I got bored typing it as a joke!
After hitting o for the sixth time on one line,
one should wonder, ever so slightly, if any of them
need to be there at all.

But this is the classic example, VERBATIM! isn't it?

--
the Poet oatmeal stout