Garbage produced

[Willem]

Kenny McCormack wrote:
) Isn't it funny how everybody has posted all this irrelevant stuff about
) sizeof, and pointers, and arrays, and what have you, but nobody has
) actually told you how to fix your program?
)
) Here's how it should be written:
)
) /* First, the usual garbage to get past the CLC censors */
) #include <stdio.h>
) int main(void)
) {
) int *x,i,n;
) double sum;
)
) printf("how many numbers? ");
) scanf("%d",&n);
) x=malloc(n*sizeof(int));
) printf("enter %d numbers\n",n);
) while(n--)scanf("%d",x+n);
) for(i=sum=0;i<n;sum+=x[i++]);
) printf("mean=%f\n",sum/n);
) }

Did you put that bug in there intentionally ?


SaSW, Willem
--
Disclaimer: I am in no way responsible for any of the statements
made in the above text. For all I know I might be
drugged or something..
No I'm not paranoid. You all think I'm paranoid, don't you !
#EOT

 

[James Lothian]

Ian Collins writes:

On 01/25/11 09:50 AM, Vikram wrote:
Hello friends

the code below will find the mean (average) of some numbers, however
it produces only garbage. Can you see a problem?

Kind Regards,
Vikram


float mean(int x[])

^^^^^^^

NB in this function I am considering the pointer, as an array.

Regardless of how you like to think about it, when you pass an array to
a function in C, the function receives a pointer to the first element of
the array. That's just the way passing an array works in C. If you take
sizeof() the parameter, you'll just get the size of the pointer, which
is nothing to do with the size of the array.

Hello James

I think you are wrong about this.

As others have said, sizeof returns a result in bytes. You haven't found a
bug in your compiler. If you want to find
out the number of items in an array, you need to divide the sizeof the array
by the sizeof an element in the array. But all this is irrelevant to the
original question: you still can't pass an array to a function and use
sizeof in the function to find out how big the array is: inside the
function, all you've got is a pointer to the first array element.

James

 

[Vikram]

Vikram wrote:
Ian Collins writes:

On 01/25/11 09:50 AM, Vikram wrote:
Hello friends

the code below will find the mean (average) of some numbers,
however it produces only garbage. Can you see a problem?

Kind Regards,
Vikram


float mean(int x[])

^^^^^^^

NB in this function I am considering the pointer, as an array.

Regardless of how you like to think about it, when you pass an array
to a function in C, the function receives a pointer to the first
element of the array. That's just the way passing an array works in C.
If you take sizeof() the parameter, you'll just get the size of the
pointer, which is nothing to do with the size of the array.

Hello James

I think you are wrong about this. I have been doing some tests and what
I discovered is my library's sizeof() function is buggy! For some
reason it miscalculates all sizes by a multiple of 4. It's easy to
correct this, see the code below

You have misunderstood.

#define sizeof(x) (sizeof(x)/4)

This macro is likely to break things very badly if you try to use it.

main()

This should be "int main(void)". Your compiler will probably let you
get away with the old-style "main()" declaration. This is not the cause
of your problems, but you should fix it anyway.

{
int a1[1];
int a2[5];
int a3[20];
printf("1=%d\n5=%d\n20=%d\n",sizeof(a1),sizeof(a2),sizeof(a3));

sizeof (either the built-in operator or your macro) yields a result of
type size_t, which is an unsigned type. The "%d" format requires an
argument of type int, a signed type. If size_t and int happen to be the
same size on your system, the above is likely to *appear* to work
correctly, but it could break badly on a different system. You can
avoid this by converting each operand to int, or (if your implementation
supports it), by using the "%zu" format which does require a size_t
argument.

printf("1=%d\n5=%d\n20=%d\n", (int)sizeof a1, (int)sizeof a2,
(int)sizeof a3);
or
printf("1=%zu\n5=%zu\n20=%zu\n", sizeof a1, sizeof a2, sizeof a3);

This is not the cause of your problems, but you should fix it anyway.

return(0);

The parentheses are unnecessary but harmless.

}

This now successfully produces:
1=1
5=5
20=20

That's the output I'd expect on a system with sizeof(int) == 4.

Some things you should understand:

sizeof is an operator, not a function. It might seem odd to have an
operator whose name is a keyword rather than a sequence of one or more
punctuation characters, but there it is.

There are actually two forms. One is ``sizeof expr'', which yields the
size of the result of an expression (without evaluating the operand in
most cases). If you write ``sizeof(a1)'', you're applying sizeof to a
parenthesized expression; the parentheses are part of the operand, not
part of the syntax of sizeof itself, just like writing ``-(x)'' rather
than ``-x''. If you're more comfortable adding parentheses, they won't
hurt anything (the same applies to your return statement), but you
should at least be able to understand what's going on when you're
reading someone else's code. In some cases you might need parentheses
just for grouping.

The other form is ``sizeof ( type-name )'', which yields the size of a
given type; for that form, the parentheses are required.

sizeof yields the size of its operand *in bytes*. It does not yield the
number of elements in an array, unless the array happens to have
one-byte elements. (A byte is *at least* 8 bits; it will be exactly 8
bits on any system you're likely to encounter.)

The reason dividing by 4 gave results that you thought were sensible is
that sizeof(int) happens to be 4 on your system. It could be 2, or 8,
or even 1, on another system (the latter is possible only if a byte is
at least 16 bits).

It's common to define a macro to determine the number of elements in an
array:

#define ARRAY_LENGTH(arr) (sizeof (arr) / sizeof (arr[0]))

Note that this works only if you apply it to an array object. If you
try to apply it to a function parameter -- well, read section 6 of the
comp.lang.c FAQ, <http://www.c-faq.com/>. And browse the rest of it
while you're there; it's an excellent resource.

Hello Kieth

Great thanks for such a clear and detailed explanation! It is a shame C
has picked up all these bad misfeatures from its history. The C-FAQ
section on pointers and arrays is very hard to understand... it would be
much better to have two kinds of pointers: Array Pointers (these would be
pointers returned by malloc and they would be 100% interchangeable with
arrays - in particular sizeof(p) = number of elements in the array) and
General-Purpose Pointers (all other pointers - these would be similar to
arrays according to the current rules (even better, according to
simplified rules) and sizeof(p) would be the number of bytes needed to
store a pointer)

Anyway another question: when I submitted my assignment, the teacher said
that instead of my code
scanf("%d",&n);
it is preferable to have an intermediate string-buffer:
scanf("%s",&buf);
n=atoi(buf);
to guard against invalid input. What can go wrong if the user inputs a
non-integer in the first case please?

Kind Regards,
Vikram

 

[Ian Collins]

Anyway another question: when I submitted my assignment, the teacher said
that instead of my code
scanf("%d",&n);
it is preferable to have an intermediate string-buffer:
scanf("%s",&buf);
n=atoi(buf);
to guard against invalid input. What can go wrong if the user inputs a
non-integer in the first case please?

The second version is undoubtedly worse than the first for at least two
reasons:

1) the string buffer can be overrun buy a very large input, a common
hacker's exploit.

2) atoi doesn't provide any error checking, so you have a more complex
solution with no added checking.

 

[Seebs]

The second version is undoubtedly worse than the first for at least two
reasons:

1) the string buffer can be overrun buy a very large input, a common
hacker's exploit.

2) atoi doesn't provide any error checking, so you have a more complex
solution with no added checking.

Agreed. However, what it *does* do is at least avoid the problem that
if you feed scanf("%d", &n) input such as "yes", it just fails and leaves
the data in the buffer.

I would usually use fgets() to read in a line, then mess with it using
strtol() or something.

-s

 

[Jens Thoms Toerring]

Great thanks for such a clear and detailed explanation! It is a shame C
has picked up all these bad misfeatures from its history. The C-FAQ
section on pointers and arrays is very hard to understand... it would be
much better to have two kinds of pointers: Array Pointers (these would be
pointers returned by malloc and they would be 100% interchangeable with
arrays - in particular sizeof(p) = number of elements in the array) and
General-Purpose Pointers (all other pointers - these would be similar to
arrays according to the current rules (even better, according to
simplified rules) and sizeof(p) would be the number of bytes needed to
store a pointer)

Be careful what you wish for, it might become true;-) You then
would need a whole bag of new syntax if you e.g. want to pass
parts of an array to a function (which is not uncommon). With
C as it is you pass a pointer plus the number of elements that
the function should deal with. But if there would be "array
pointers" how would you propose to convert such a pointer that
points to an array of say 100 elements to one that points to
only 10, starting at the 42nd element? And then, for program-
ming near to the metal, where one often deals with hardware
memory addresses, you would need some syntax to be able to
not only assign an address to the pointer but also how many
bytes/elements in memory should be considered part of the array.
I feel you would end up with a lot of extra, ugly syntax with
not too many real advantages.

But then it's easy to set up your own "counted array pointers".
Instead of using a normal array use a structure like

typedef {
int * data;
size_t len;
} intArray;

Assign what you got from malloc() (or a normal array) to 'data'
and the number of elements to 'len' and pass that to a function
(either directly or as a pointer) and, voila, you've got a
"counted array" at the cost of just a tiny bit of extra typing.
C isn't a large language (and that's one of it's advantages)
but there's nothing that keeps you from creating larger logical
structures out of the elementary building blocks that better
suite your needs.

Of course, feel free to use e.g. C++ instead of C with e.g.
STL vectors - they have exactly the behaviour you're looking
for. Or use some library (the "container library" Jacob Navia
wrote and discussed here quite a number of times in this very
group comes to mind).

Anyway another question: when I submitted my assignment, the teacher said
that instead of my code
scanf("%d",&n);
it is preferable to have an intermediate string-buffer:
scanf("%s",&buf);
n=atoi(buf);
to guard against invalid input. What can go wrong if the user inputs a
non-integer in the first case please?

This is definitely not a good idea. There are several reasons:

a) It's unnecessary - scanf() protects against invalid input.
You just have to check scanf()'s return value, which is the
number of items read in. If it's short of what you expected
there was invalid input. In your case, if scanf() doesn't
return 1 then what the user entered wasn't an integer.

b) You need an extra buffer. How large does it need to be? That's
not a trivial question. It can be hard enough even if you can
expect reasonable input. And it becomes impossible when the
user enters e.g. "0000000000000000000000000000000000001" while
you expect a simple, short number. But that's perfectly valid
input and 'scanf("%d",&n)' deals with it without problems. So,
how long do you need to make the input buffer to cater for all
possible (and valid) cases?

But, even without considering that problem, what your teacher
proposed is very bad (at least if it's exactly as you wrote)
because it might result in a buffer overflow. A format speci-
fier of "%s" doesn't tell scanf() when to stop reading and thus
scanf() will read without regard for how much room there is in
the buffer until it finds a white-space character in the input
- in the process possibly writing past the end of the buffer!
You would at least need something like

scanf( "%99s", buf );

for the case that 'buf' is a char array with 100 elements.

Not telling scanf() how much it is allowed to read into a
char buffer is a very bad mistake. Never ever use a bare
"%s" with scanf() (unless you're absolutely, totally sure
what to expect as input, but even then...).

c) Even if we avoided all those problems using atoi() still is
another thing one should avoid. The problem is that atoi()
(in contrast to scanf()) in no way can tell you if things
worked out as expected. If you passed a buffer to it that
didn't start (after an arbitrary number of white-space cha-
racters) with a number it will return 0, which looks like
reasonable input and gives you no indication that there actu-
ally was a problem. Always use strtol() (for integers) instead.

So, in my opinion, what your teacher proposed is much worse than
what you started with: it makes things a lot more complicated (or
even impossible to get right), it opens up a security hole and
you end up knowing nothing more about the validity of the input.

Regards, Jens

 

[Michael Press]

Hello friends

the code below will find the mean (average) of some numbers, however it
produces only garbage. Can you see a problem?

float mean(int x[])
{
int i,n;
float sum;
n=sizeof(x);
for(i=sum=0;i<n;sum+=x[i++]);
return sum/n;

Remember to check for n == 0.
This can be awkward to handle.
Do you need to alert the caller
to this circumstance? Then you
need to recast the return mechanism.
Or you can simply return 0.0
or FLT_MAX when n == 0.