memset doesn't work as expected

[thomas]

I allocated a piece of memory and use memset to set it to 0.
------------------------------------
int *graph = new int[16];
memset(graph, 0, sizeof(graph));
for(int i=0;i<4;i++){
for(int j=0;j<4;j++)
cout<<graph[i*n+j]<<" ";
cout<<endl;
}

 

[thomas]

or can I new a piece of memory and set it to zero in the mean time?

 

[Lars Uffmann]

I allocated a piece of memory and use memset to set it to 0.

sizeof (graph) should be sizeof (int *) - which is definitely not sizeof
(int)*16. So you are only setting the first sizeof(int *) bytes in the
array to zero.

Try memset (graph, 0, sizeof (int)*16) instead.

 

[peter koch]

I allocated a piece of memory and use memset to set it to 0.
------------------------------------
int *graph = new int[16];
memset(graph, 0, sizeof(graph));
for(int i=0;i<4;i++){
    for(int j=0;j<4;j++)
        cout<<graph[i*n+j]<<" ";
    cout<<endl;}

This is because you do not use std::vector. Always use high-level
constructs unless you have a good reason not to: low level programming
requires you to take care of lots of details that are irrelevant to
your problem and might be difficult to get right. One of your problems
here is that sizeof did not return what you thought, but there are
other problems lurking!

/Peter

 

[Lars Uffmann]

This is because you do not use std::vector. Always use high-level
constructs unless you have a good reason not to: low level programming
requires you to take care of lots of details that are irrelevant to
your problem and might be difficult to get right.

I fail to see a problem other than getting the size right in this
case... Isn't speed always a "good reason" to do low level programming?
I am somewhat estranged here by your general "always use high-level
constructs" statement.

One of your problems here is that sizeof did not return what you
thought, but there are other problems lurking!

Care to enlighten me, for one? I'm curious. Assuming he has set n to
4 first and should synchronize n with the max value for j, and should
probably link the size of the array construction to that with a
const/variable, I currently see no other problems...

Best Regards,

Lars

 

[Lionel B]

I allocated a piece of memory and use memset to set it to 0.
------------------------------------
int *graph = new int[16];
memset(graph, 0, sizeof(graph));
for(int i=0;i<4;i++){
    for(int j=0;j<4;j++)
        cout<<graph[i*n+j]<<" ";
    cout<<endl;}

This is because you do not use std::vector. Always use high-level
constructs unless you have a good reason not to:

Steady on, that could come across as somewhat patronising. You do not
know for sure that the OP does not have a "good reason not to use a high-
level construct". Ok, maybe unlikely in this case - but then again, to
learn about (the dangers of) low-level constructs might be a valid reason
to try them out and, as the OP has done, get some feedback on the results.

low level programming
requires you to take care of lots of details that are irrelevant to your
problem and might be difficult to get right.

Again: you don't know what the OP is trying to achieve (they don't say).

 

[Kai-Uwe Bux]

I fail to see a problem other than getting the size right in this
case... Isn't speed always a "good reason" to do low level programming?

No, there are many cases where speed is not a good reason to engage in low
level programming.

I am somewhat estranged here by your general "always use high-level
constructs" statement.

Well, if you leave out the "unless you have a good reason not to" part, the
statement is false.


[snip]


Best

Kai-Uwe Bux

 

[Richard Herring]

I fail to see a problem other than getting the size right in this
case... Isn't speed always a "good reason" to do low level programming?

Not if it conflicts with clarity and maintainability. Less still when
it's probably fictional (it's unlikely that initialising a vector of
ints takes any longer than using new[] and memset.)

In any case, first you'd have to show (a) that the high-level solution
is too slow, (b) the low-level solution is actually faster, and (c) that
the speed increase actually has a measurable effect on the performance
of the program as a whole.

I am somewhat estranged here by your general "always use high-level
constructs" statement.


Care to enlighten me, for one? I'm curious. Assuming he has set n to
4 first and should synchronize n with the max value for j, and
should probably link the size of the array construction to that with a
const/variable,

That's three assumptions already ;-)

I currently see no other problems...

Until he decides to switch from int to some user-defined type that isn't
POD...

 

[Lars Uffmann]

That's three assumptions already ;-)

Yeah, I know... I sorta objected first, then discovered one issue after
another *g* But none that would actually make the code not work if
treated properly. So I thought maybe I was missing something.

Until he decides to switch from int to some user-defined type that isn't
POD...

What does POD mean?

Do you mean some type that'll only store pointers to it's data in the
array, or some type where sizeof (type) doesn't yield the correct size?

Best Regards,

Lars

 

[Richard Herring]

Yeah, I know... I sorta objected first, then discovered one issue after
another *g* But none that would actually make the code not work if
treated properly. So I thought maybe I was missing something.

What does POD mean?

Plain Old Data, though on looking more closely what I meant isn't
exactly what the standard defines as POD.

Do you mean some type that'll only store pointers to it's data in the
array, or some type where sizeof (type) doesn't yield the correct size?

I meant some type where all-bits-zero doesn't equate to having value
zero, or whose constructor actually needs to do something.

 

[werasm]

I allocated a piece of memory and use memset to set it to 0.
------------------------------------
int *graph = new int[16];
memset(graph, 0, sizeof(graph));
for(int i=0;i<4;i++){
for(int j=0;j<4;j++)
cout<<graph[i*n+j]<<" ";
cout<<endl;}

Even if speed was an issue and you could not
use vector, I would nevertheless not use
memset if I were you. What's wrong with
std::fill? <memset> is errorprone in more
than one way.

void* memset(void* s, int c, size_t n);

- firstly <n> is specified in bytes and not in items.
- <s> is not type safe.
- <c> is converted to an unsigned char, hence you may
not get what you expect (zero(0) as argument is fine,
of course).

Another reason why I steer away from these functions
is because I've found that in large projects they most
often caused serious problems. I used to search for
memset, memcpy, strncpy, sprintf, printf, and strcpy,
then check each one individually (or simply replace them)
whereafter most of the problems would disappear.

For what you are doing (You may have omitted some info)
the simplest would be:

std::vector<int> graph( 16, 0 );

If you have no alternative, I would consider using the stack
prior to heap if you know the size at compile time - therefore:

int graph[SomeCompileTimeConstant] = { 0 };

int *graph = new int[16];
memset(graph, 0, sizeof(graph));

If you have to use the heap and you cannot use vector (for
which reason for the life of it I cannot see), then you have
another problem in this case, which is that the memory is
more than likely not freed at the end of scope (I'm saying
more than likely because in some cases the application might
end in which case it does not necessarily matter). Aside
from that problem I would then nevertheless use std::fill:

int *graph = new int[someVariable];
std::fill( graph, graph+someVariable, 0 );

Now for the reason for your problem, compiling the
little program below on http://www.comeaucomputing.com/tryitout/
shows you what you can expect from sizeof. From this you can
see that graph is in actual fact a pointer to an integer (the
first integer in an array) and not an array. You will also
notice that the size of a pointer may differ from the size of
an array, depending on the amount of elements in the array
and the size of each element.

#include <algorithm>
#include <cassert>

int main()
{
int* graph = new int[16];

assert( sizeof( graph ) == sizeof(int*) );
assert( sizeof(int*) != sizeof(int[16]) );

static_assert( sizeof( graph ) == sizeof(int*), "" );
static_assert( sizeof(int*) != sizeof(int[16]), "" );
}

Kind regards,

Werner

 

[Andrew Koenig]

I allocated a piece of memory and use memset to set it to 0.

Don't use memset. Your example shows one good reason: sizeof(graph) is the
size of a pointer to int (because that's what graph is), so you will set to
zero a number of bytes equal to the size of a pointer, which may or may not
be the size of an int.

If you insist on using new/delete instead of a vector, here's a cleaner way
to do what you want:

int *graph = new int[16];
std::fill(graph, 16, 0);

Note that you cannot use sizeof(graph) instead of 16. You can, however, do
the following to avoid having to write 16 more than once:

size_t graph_size = 16;
int *graph = new int[graph_size];
std::fill(graph, graph_size, 0);

 

[Lars Uffmann]

Plain Old Data, though on looking more closely what I meant isn't
exactly what the standard defines as POD.

*pling* a-haaa

I meant some type where all-bits-zero doesn't equate to having value
zero, or whose constructor actually needs to do something.

Oh, okay, I get it. Thanks - I'm too much used to i386 architectures

Thanks!

Lars

 

[red floyd]

"thomas" <[email protected]> wrote in message

I can't believe I'm correcting Andrew Koenig!!!!

Don't use memset. Your example shows one good reason: sizeof(graph) is the
size of a pointer to int (because that's what graph is), so you will set to
zero a number of bytes equal to the size of a pointer, which may or may not
be the size of an int.

If you insist on using new/delete instead of a vector, here's a cleaner way
to do what you want:

int *graph = new int[16];
std::fill(graph, 16, 0);

std::fill_n(graph, 16, 0);
or
std::fill(graph, graph+16, 0);

Note that you cannot use sizeof(graph) instead of 16. You can, however, do
the following to avoid having to write 16 more than once:

size_t graph_size = 16;
int *graph = new int[graph_size];
std::fill(graph, graph_size, 0);

again:

std::fill_n(graph, graph_size, 0);
or
std::fill(graph, graph+graph_size, 0);

 

[N1GHTS]

The "new[]" operator uses "sizeof" internally to calculate the size of the type given to it so it can allocate the user defined multiple of that size of space in the heap. To use "sizeof" to analyze the size of the allocated heap under the pointer name "graph" is no good because "graph" is just a pointer and in itself contains no specific internal reference to its fully allocated size. What "sizeof(graph)" will do is return the size of "graph[0]" which is the same as "sizeof(int)" which happens to be 4 in standard 32 bit computers.

To avoid this issue, create a variable or constant in an object/struct or globally that will record the size of your graph. And don't forget to "delete[] graph" when you are done to avoid a memory leak.

BTW, I was quite amused by the follow-up comment made to the initial post which claimed that the reason your code didn't work is because you didn't use std::vector. Its exactly the same as saying "the reason your bicycle isn't working is because you are not using a car." There is nothing wrong with using C-style language in C++. Obviously to use std::vector has its exclusive advantages, especially in its ability to let you concentrate on higher level tasks without sacrificing additional programming time. Personally I favor lower level control of programming because not only does the programmer understand how your code works close to the processor level, you can catch optimization flaws much more quickly and of course there is less overhead on refined code. Still, even the best low level programmers have their share of mistakes that could have been prevented using the fullest capability of C++ which include high level constructs, and those mistakes take time away from progress... so you could still say "if you use a car, you will get there faster than a bicycle, but lose out on the ability to use shortcuts that a car can't fit into. As for why the bicycle doesn't work, the chain was disconnected."

 

[peter koch]

I allocated a piece of memory and use memset to set it to 0.
------------------------------------
int *graph = new int[16];
memset(graph, 0, sizeof(graph));
for(int i=0;i<4;i++){
    for(int j=0;j<4;j++)
        cout<<graph[i*n+j]<<" ";
    cout<<endl;}

This is because you do not use std::vector. Always use high-level
constructs unless you have a good reason not to:

Steady on, that could come across as somewhat patronising. You do not
know for sure that the OP does not have a "good reason not to use a high-
level construct".

I do know for sure. For the post of Thomas shows beyond doubt that he
is a beginner using C++ and probably also a beginner wrt programming.

Ok, maybe unlikely in this case - but then again, to
learn about (the dangers of) low-level constructs might be a valid reason
to try them out and, as the OP has done, get some feedback on the results.

I did give him feedback. Not only the important one (to stay away from
low-level stuff) but also why his program did not work.

Again: you don't know what the OP is trying to achieve (they don't say).

I might not know what the OP is trying to achieve in details, but if
it is not related to the problem he is asking there's something fishy
going on!

/Peter

 

[Bo Persson]

I allocated a piece of memory and use memset to set it to 0.
------------------------------------
int *graph = new int[16];
memset(graph, 0, sizeof(graph));
for(int i=0;i<4;i++){
for(int j=0;j<4;j++)
cout<<graph[i*n+j]<<" ";
cout<<endl;}

This is because you do not use std::vector. Always use high-level
constructs unless you have a good reason not to:

Steady on, that could come across as somewhat patronising. You do
not know for sure that the OP does not have a "good reason not to
use a high- level construct". Ok, maybe unlikely in this case - but
then again, to learn about (the dangers of) low-level constructs
might be a valid reason to try them out and, as the OP has done,
get some feedback on the results.

Trying to optimize code by using this kind of low level constructs,
assumes that the OP can easily outsmart the library implementor. What
are the odds for that?


Bo Persson

 

[James Kanze]

In message <[email protected]>, Lars Uffmann
<[email protected]> writes

[...]

Until he decides to switch from int to some user-defined type
that isn't POD...

Theoretically, at least, memset of 0 doesn't even work for all
POD types. The only thing it's guaranteed to work for are
integral types (and I'm not even sure about those---int's can
have padding bits as well).

In practice, there have been machines where null pointers
didn't have all bits 0.

 

[James Kanze]

I allocated a piece of memory and use memset to set it to 0.
------------------------------------
int *graph = new int[16];
memset(graph, 0, sizeof(graph));
for(int i=0;i<4;i++){
for(int j=0;j<4;j++)
cout<<graph[i*n+j]<<" ";
cout<<endl;}

This is because you do not use std::vector. Always use high-level
constructs unless you have a good reason not to:

Steady on, that could come across as somewhat patronising.

I think it comes across more as somewhat professional. The code
above does exactly what std::vector does (except that
std::vector does it correctly).

 

[Lionel B]

I allocated a piece of memory and use memset to set it to 0.
------------------------------------
int *graph = new int[16];
memset(graph, 0, sizeof(graph));
for(int i=0;i<4;i++){
for(int j=0;j<4;j++)
cout<<graph[i*n+j]<<" ";
cout<<endl;}
--------------------------------
But I found that the output is really strange -- graph[0][1] is
always a large number.
I expected the output will all be 0.
This is because you do not use std::vector. Always use high-level
constructs unless you have a good reason not to:

Steady on, that could come across as somewhat patronising.

I think it comes across more as somewhat professional.

Like he gets paid to post here? ;-)

The code above
does exactly what std::vector does (except that std::vector does it
correctly).

Of course. I wasn't disputing the advice, more the tone, which I thought
a bit snarky. Anyhow, no big deal.