###Other comments:
Other comments:
#This code is so full of bugs. It has no write to work.
This code is so full of bugs. It has no write to work.
###Other comments:
#This code is so full of bugs. It has no write to work.
Other comments:
This code is so full of bugs. It has no write to work.
Since you are using C++ compiler. You should use std::vector<viByte>
or std::basic_string<viByte>
.
This
This kind of bug is easy to introduce and unless you have good unit test hard to find. So why not use a class that already is unit test and has had 10 years of millions of people lookign at it to make sure it works correctly.
###Other comments:
Broken and illegal.
buf->str = ( viByte* ) 0x1;
Wrong. buf->str is a pointer and it was expecting a unsigned integer (they are not the same). Use %p to print a pointer value.
printf(" Before viByteBuf_Prepend => %uc ", buf->str);
Broken and illegal (0x2 is not yours)
viByteBuf_Prepend( buf, 3, ( viByte* ) 0x2 );
No check for NULL. (calloc can return NULL when it fails).
viByteBuf* buf = ( viByteBuf* ) calloc( sizeof( viByteBuf ), 1 );
Again no check for NULL.
buf->str = ( viByte* ) calloc( sizeof( viByte ), buflen );
Is this really what you want? Buflen includes the portion with the '0円' character. You need to be consistent and decide if len does or does not include the null.
buf->len = buflen;
As pointed out above (write past the end of the buffer).
buf->str[ buflen ] = '0円';
How do you handle failure?
As pointed out above calloc can fail. How do you propagate that failure to the caller.
viByteBuf* tmp = viByteBuf_New( totallen );
Why are you nulling the whole array. You are just about to write over it.
Seems like a complete waste of time.
memset( tmp->str, 0, tmp->len );
Looks like this loop is trying to reverse the string in place.
Which will fail as by the time you overwrite the first half the second half is a copy of the back half. Luckily it completely fails because you don't actually use the index
variable.
for( i = 0; i < buf->len; ++i )
{
index = ( buf->len - i ) - 1;
*strpos = buf->str[ 0 ];
++strpos;
}
Now that you have failed to reverse it. You then overwrite it again with 0 (presumably to cover up the failure to reverse it).
memset( buf->str, 0, buf->len );
Illegal parameter to printf() again.
printf( "%uc\n", buf->str );
More copying for unknow reason.
memcpy( buf->str, tmp->str, tmp->len );
#This code is so full of bugs. It has no write to work.
Since you are using C++ compiler. You should use std::vector<viByte>
.
This kind of bug is easy to introduce and unless you have good unit test hard to find. So why not use a class that already is unit test and has had 10 years of millions of people lookign at it to make sure it works correctly.
Since you are using C++ compiler. You should use std::vector<viByte>
or std::basic_string<viByte>
.
This kind of bug is easy to introduce and unless you have good unit test hard to find. So why not use a class that already is unit test and has had 10 years of millions of people lookign at it to make sure it works correctly.
###Other comments:
Broken and illegal.
buf->str = ( viByte* ) 0x1;
Wrong. buf->str is a pointer and it was expecting a unsigned integer (they are not the same). Use %p to print a pointer value.
printf(" Before viByteBuf_Prepend => %uc ", buf->str);
Broken and illegal (0x2 is not yours)
viByteBuf_Prepend( buf, 3, ( viByte* ) 0x2 );
No check for NULL. (calloc can return NULL when it fails).
viByteBuf* buf = ( viByteBuf* ) calloc( sizeof( viByteBuf ), 1 );
Again no check for NULL.
buf->str = ( viByte* ) calloc( sizeof( viByte ), buflen );
Is this really what you want? Buflen includes the portion with the '0円' character. You need to be consistent and decide if len does or does not include the null.
buf->len = buflen;
As pointed out above (write past the end of the buffer).
buf->str[ buflen ] = '0円';
How do you handle failure?
As pointed out above calloc can fail. How do you propagate that failure to the caller.
viByteBuf* tmp = viByteBuf_New( totallen );
Why are you nulling the whole array. You are just about to write over it.
Seems like a complete waste of time.
memset( tmp->str, 0, tmp->len );
Looks like this loop is trying to reverse the string in place.
Which will fail as by the time you overwrite the first half the second half is a copy of the back half. Luckily it completely fails because you don't actually use the index
variable.
for( i = 0; i < buf->len; ++i )
{
index = ( buf->len - i ) - 1;
*strpos = buf->str[ 0 ];
++strpos;
}
Now that you have failed to reverse it. You then overwrite it again with 0 (presumably to cover up the failure to reverse it).
memset( buf->str, 0, buf->len );
Illegal parameter to printf() again.
printf( "%uc\n", buf->str );
More copying for unknow reason.
memcpy( buf->str, tmp->str, tmp->len );
#This code is so full of bugs. It has no write to work.
Your problem is here:
buf->str[ buflen ] = '0円';
You allocate a buffer (str) of buflen bytes.
This means you can index it (str) from 0 -> (buflen-1).
Thus the accesses above is writing one past the end of the buffer and corrupting memory.
You probably meant:
buf->str[ len ] = '0円';
Since you are using C++ compiler. You should use std::vector<viByte>
.
This kind of bug is easy to introduce and unless you have good unit test hard to find. So why not use a class that already is unit test and has had 10 years of millions of people lookign at it to make sure it works correctly.