Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > comp.lang.c > #15077 > unrolled thread

Memory leaking in memmove

Started byArifulHossain tuhin <etothepowerpi@gmail.com>
First post2011-12-28 01:24 -0800
Last post2011-12-28 10:36 -0800
Articles 17 — 8 participants

Back to article view | Back to comp.lang.c


Contents

  Memory leaking in memmove ArifulHossain tuhin <etothepowerpi@gmail.com> - 2011-12-28 01:24 -0800
    Re: Memory leaking in memmove Nick Keighley <nick_keighley_nospam@hotmail.com> - 2011-12-28 02:20 -0800
      Re: Memory leaking in memmove ArifulHossain tuhin <etothepowerpi@gmail.com> - 2011-12-28 02:37 -0800
        Re: Memory leaking in memmove Philip Lantz <prl@canterey.us> - 2011-12-28 02:42 -0800
        Re: Memory leaking in memmove Nick Keighley <nick_keighley_nospam@hotmail.com> - 2011-12-28 03:36 -0800
          Re: Memory leaking in memmove ArifulHossain tuhin <etothepowerpi@gmail.com> - 2011-12-28 03:50 -0800
        Re: Memory leaking in memmove Seebs <usenet-nospam@seebs.net> - 2011-12-28 18:17 +0000
    Re: Memory leaking in memmove Philip Lantz <prl@canterey.us> - 2011-12-28 02:36 -0800
      Re: Memory leaking in memmove ArifulHossain tuhin <etothepowerpi@gmail.com> - 2011-12-28 03:57 -0800
        Re: Memory leaking in memmove Dr Nick <3-nospam@temporary-address.org.uk> - 2011-12-28 12:21 +0000
          Re: Memory leaking in memmove ArifulHossain tuhin <etothepowerpi@gmail.com> - 2011-12-28 04:26 -0800
        Re: Memory leaking in memmove jt@toerring.de (Jens Thoms Toerring) - 2011-12-28 12:46 +0000
    Re: Memory leaking in memmove ArifulHossain tuhin <etothepowerpi@gmail.com> - 2011-12-28 05:54 -0800
    Re: Memory leaking in memmove Ben Bacarisse <ben.usenet@bsb.me.uk> - 2011-12-28 14:46 +0000
      Re: Memory leaking in memmove Philip Lantz <prl@canterey.us> - 2011-12-28 12:20 -0800
        Re: Memory leaking in memmove Ben Bacarisse <ben.usenet@bsb.me.uk> - 2011-12-28 21:50 +0000
    Re: Memory leaking in memmove Barry Schwarz <schwarzb@dqel.com> - 2011-12-28 10:36 -0800

#15077 — Memory leaking in memmove

FromArifulHossain tuhin <etothepowerpi@gmail.com>
Date2011-12-28 01:24 -0800
SubjectMemory leaking in memmove
Message-ID<15848343.42.1325064299310.JavaMail.geo-discussion-forums@prdv33>
My application is leaking memory in a memmove call. I have made it sure from coredumps. 

the offending code snippet reads as follows:
unsigned char pad_len;
pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);
if(pad_len%30 == 0)
pad_len = 9;
if(packet->size > 0){
	memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);
	packet->size += pad_len;
	packet->data.buf[0] = pad_len;
	for(j=1; j< pad_len; j++)
	packet->data.buf[j] = (char)256%j;
	EnDeCrypt(packet->data.buf, packet->size, key, KEYLEN);
}

the packet data structure is decalared as follows:

struct rtp_packet {
    size_t      size;
    ................
    (some fields)
    ................
    struct rtp_packet *next;
    struct rtp_packet *prev;
    union {
	rtp_hdr_t       header;
	unsigned char   buf[8192];
    } data;
}; 

As the data.buf is statically allocated, valgrind does not complain anything about it.
Is it running out of memory? or any other problem?
Any suggestion?

[toc] | [next] | [standalone]


#15079

FromNick Keighley <nick_keighley_nospam@hotmail.com>
Date2011-12-28 02:20 -0800
Message-ID<c74eb109-792e-4d93-91c9-842b966907f4@f33g2000yqh.googlegroups.com>
In reply to#15077
On Dec 28, 9:24 am, ArifulHossain tuhin <etothepowe...@gmail.com>
wrote:
> My application is leaking memory in a memmove call. I have made it sure from coredumps.
>
> the offending code snippet reads as follows:
> unsigned char pad_len;
> pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);
> if(pad_len%30 == 0)
> pad_len = 9;
> if(packet->size > 0){
>         memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);
>         packet->size += pad_len;
>         packet->data.buf[0] = pad_len;
>         for(j=1; j< pad_len; j++)
>         packet->data.buf[j] = (char)256%j;
>         EnDeCrypt(packet->data.buf, packet->size, key, KEYLEN);
>
> }
>
> the packet data structure is decalared as follows:
>
> struct rtp_packet {
>     size_t      size;
>     ................
>     (some fields)
>     ................
>     struct rtp_packet *next;
>     struct rtp_packet *prev;
>     union {
>         rtp_hdr_t       header;
>         unsigned char   buf[8192];
>     } data;
>
> };
>
> As the data.buf is statically allocated, valgrind does not complain anything about it.
> Is it running out of memory?

well? is it? How do you know you have  memory leak?

> or any other problem?
> Any suggestion?

I can't really see how this leaks. Where does packet come from?

[toc] | [prev] | [next] | [standalone]


#15080

FromArifulHossain tuhin <etothepowerpi@gmail.com>
Date2011-12-28 02:37 -0800
Message-ID<23252303.950.1325068627929.JavaMail.geo-discussion-forums@preg25>
In reply to#15079
The app crashes with seg fault. Coredump's backtrace shows it occurs in memove() statements. 

The packet comes from network, its a RTP packet embedded in UDP payload.

[toc] | [prev] | [next] | [standalone]


#15083

FromPhilip Lantz <prl@canterey.us>
Date2011-12-28 02:42 -0800
Message-ID<MPG.2964927c90ec2a17989681@news.eternal-september.org>
In reply to#15080
ArifulHossain tuhin wrote:
> 
> The app crashes with seg fault. Coredump's backtrace shows it occurs
> in memove() statements.
> 
> The packet comes from network, its a RTP packet embedded in UDP payload.

packet->size must be out of range. You should check that it is a valid 
value. Perhaps it was -1, indicating an error, which was converted to 
size_t, yielding a very large number.

[toc] | [prev] | [next] | [standalone]


#15095

FromNick Keighley <nick_keighley_nospam@hotmail.com>
Date2011-12-28 03:36 -0800
Message-ID<bca9950e-3fb2-474e-9cbd-3ed05c258839@z12g2000yqm.googlegroups.com>
In reply to#15080
On Dec 28, 10:37 am, ArifulHossain tuhin <etothepowe...@gmail.com>
wrote:

> The app crashes with seg fault. Coredump's backtrace shows it occurs in memove() statements.

that doesn't sound like a memory leak to me. Print (or use a debugger)
the values of the parameters passed to the memmove(). In fact why not
just use a debugger? A debugger can also be used on a core dump.

> The packet comes from network, its a RTP packet embedded in UDP payload.

I meant, how was it allocated.

[toc] | [prev] | [next] | [standalone]


#15098

FromArifulHossain tuhin <etothepowerpi@gmail.com>
Date2011-12-28 03:50 -0800
Message-ID<19848468.838.1325073016070.JavaMail.geo-discussion-forums@prlu4>
In reply to#15095
On Wednesday, December 28, 2011 5:36:59 PM UTC+6, Nick Keighley wrote:
> On Dec 28, 10:37 am, ArifulHossain tuhin <etothe...@gmail.com>
> wrote:
> 
> > The app crashes with seg fault. Coredump's backtrace shows it occurs in memove() statements.
> 
> that doesn't sound like a memory leak to me.

Sorry for the mistake. Actually its "SegFaulting".
> Print (or use a debugger)
> the values of the parameters passed to the memmove(). In fact why not
> just use a debugger? A debugger can also be used on a core dump.
> 

I have used gdb to print the parameters passed to memove(). But i'm unsure what to do with it. Any suggestion ? what should i do with the parameters?

> > The packet comes from network, its a RTP packet embedded in UDP payload.
> 
> I meant, how was it allocated.

rtp_packet is allocated dynamically. But the data.buf field is allocated as you see as static allocation.

[toc] | [prev] | [next] | [standalone]


#15123

FromSeebs <usenet-nospam@seebs.net>
Date2011-12-28 18:17 +0000
Message-ID<slrnjfmlvh.2lac.usenet-nospam@guild.seebs.net>
In reply to#15080
On 2011-12-28, ArifulHossain tuhin <etothepowerpi@gmail.com> wrote:
> The app crashes with seg fault. Coredump's backtrace shows it occurs in memove() statements. 

I think you confused us by calling this a "memory leak".  Which it's not.

A memory leak is where you allocate memory and lose the pointer to it
so you can't free it.

-s
-- 
Copyright 2011, all wrongs reversed.  Peter Seebach / usenet-nospam@seebs.net
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.

[toc] | [prev] | [next] | [standalone]


#15081

FromPhilip Lantz <prl@canterey.us>
Date2011-12-28 02:36 -0800
Message-ID<MPG.29649112ea89c5de989680@news.eternal-september.org>
In reply to#15077
ArifulHossain tuhin wrote:
> My application is leaking memory in a memmove call. I have made it
> sure from coredumps.
I don't think you mean "leaking memory". That term specifically refers 
to not freeing memory that was allocated. I'm guessing you mean that 
your program is overwriting memory that it shouldn't.

> the offending code snippet reads as follows:
> unsigned char pad_len;
> pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);
> if(pad_len%30 == 0)
> pad_len = 9;
> if(packet->size > 0){
> 	memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);
> 	packet->size += pad_len;
> 	packet->data.buf[0] = pad_len;
> 	for(j=1; j< pad_len; j++)
> 	packet->data.buf[j] = (char)256%j;
> 	EnDeCrypt(packet->data.buf, packet->size, key, KEYLEN);
> }
> 
> As the data.buf is statically allocated, valgrind does not complain
> anything about it.
> Is it running out of memory? or any other problem?
> Any suggestion?


There is no dynamic memory allocation in the fragment you have shown us, 
so it can't be running out of memory.

Are you sure that packet->size + pad_len < sizeof packet->data.buf? That 
check should probably be in the code, unless there are constraints on 
the packet size that guarantee that it is always true.

[toc] | [prev] | [next] | [standalone]


#15100

FromArifulHossain tuhin <etothepowerpi@gmail.com>
Date2011-12-28 03:57 -0800
Message-ID<31200333.963.1325073432055.JavaMail.geo-discussion-forums@prez15>
In reply to#15081
On Wednesday, December 28, 2011 4:36:37 PM UTC+6, Philip Lantz wrote:
> ArifulHossain tuhin wrote:
> > My application is leaking memory in a memmove call. I have made it
> > sure from coredumps.
> I don't think you mean "leaking memory". That term specifically refers 
> to not freeing memory that was allocated. I'm guessing you mean that 
> your program is overwriting memory that it shouldn't.
> 
> > the offending code snippet reads as follows:
> > unsigned char pad_len;
> > pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);
> > if(pad_len%30 == 0)
> > pad_len = 9;
> > if(packet->size > 0){
> > 	memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);
> > 	packet->size += pad_len;
> > 	packet->data.buf[0] = pad_len;
> > 	for(j=1; j< pad_len; j++)
> > 	packet->data.buf[j] = (char)256%j;
> > 	EnDeCrypt(packet->data.buf, packet->size, key, KEYLEN);
> > }
> > 
> > As the data.buf is statically allocated, valgrind does not complain
> > anything about it.
> > Is it running out of memory? or any other problem?
> > Any suggestion?
> 
> 
> There is no dynamic memory allocation in the fragment you have shown us, 
> so it can't be running out of memory.
> 

You are absolutely right. I guess i'm misinformed about terminologies. Thanks

> Are you sure that packet->size + pad_len < sizeof packet->data.buf? That 
> check should probably be in the code, unless there are constraints on 
> the packet size that guarantee that it is always true.

Packet size can be as high as 230 with pad_len. pad_len is unsigned char. so it should cover upto 256. sizeof(packet->data.buf) is 8 so it should not be a problem. Though i guess should put a checking into the code anyway.

[toc] | [prev] | [next] | [standalone]


#15103

FromDr Nick <3-nospam@temporary-address.org.uk>
Date2011-12-28 12:21 +0000
Message-ID<87vcp0ga4w.fsf@temporary-address.org.uk>
In reply to#15100
ArifulHossain tuhin <etothepowerpi@gmail.com> writes:

> On Wednesday, December 28, 2011 4:36:37 PM UTC+6, Philip Lantz wrote:
>> ArifulHossain tuhin wrote:
>> > My application is leaking memory in a memmove call. I have made it
>> > sure from coredumps.
>> I don't think you mean "leaking memory". That term specifically refers 
>> to not freeing memory that was allocated. I'm guessing you mean that 
>> your program is overwriting memory that it shouldn't.
>> 
>> > the offending code snippet reads as follows:
>> > unsigned char pad_len;
>> > pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);
>> > if(pad_len%30 == 0)
>> > pad_len = 9;
>> > if(packet->size > 0){
>> > 	memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);
>> > 	packet->size += pad_len;
>> > 	packet->data.buf[0] = pad_len;
>> > 	for(j=1; j< pad_len; j++)
>> > 	packet->data.buf[j] = (char)256%j;
>> > 	EnDeCrypt(packet->data.buf, packet->size, key, KEYLEN);
>> > }
>> > 
>> > As the data.buf is statically allocated, valgrind does not complain
>> > anything about it.
>> > Is it running out of memory? or any other problem?
>> > Any suggestion?
>> 
>> 
>> There is no dynamic memory allocation in the fragment you have shown us, 
>> so it can't be running out of memory.
>> 
>
> You are absolutely right. I guess i'm misinformed about terminologies. Thanks
>
>> Are you sure that packet->size + pad_len < sizeof packet->data.buf? That 
>> check should probably be in the code, unless there are constraints on 
>> the packet size that guarantee that it is always true.
>
> Packet size can be as high as 230 with pad_len. pad_len is unsigned char. so it should cover upto 256. sizeof(packet->data.buf) is 8 so it should not be a problem. Though i guess should put a checking into the code anyway.

How long is your actual packet buffer? - you say above that it's 8 but
you might be misunderstanding the question.  If not, then you'll have
problems.

If at any time packet_size + pad_len is longer than the size of
packet->data.buf then you'll be scribbling on memory you don't own.

Can you show us the code that defines and sets packet->data.buf

As packet->data.buf must contain at least as many bytes as the maximum
length of pad_len, can you tell us what RANSTOP and RANSTART are
#define'd to?

It might be worth printing pad_len just before the memmove and seeing
what the value is when it blows up.
-- 
Online waterways route planner            | http://canalplan.eu
Plan trips, see photos, check facilities  | http://canalplan.org.uk

[toc] | [prev] | [next] | [standalone]


#15104

FromArifulHossain tuhin <etothepowerpi@gmail.com>
Date2011-12-28 04:26 -0800
Message-ID<13831642.279.1325075201100.JavaMail.geo-discussion-forums@prfi34>
In reply to#15103
On Wednesday, December 28, 2011 6:21:03 PM UTC+6, Dr Nick wrote:
> ArifulHossain tuhin <etothe...@gmail.com> writes:
> 
> > On Wednesday, December 28, 2011 4:36:37 PM UTC+6, Philip Lantz wrote:
> >> ArifulHossain tuhin wrote:
> >> > My application is leaking memory in a memmove call. I have made it
> >> > sure from coredumps.
> >> I don't think you mean "leaking memory". That term specifically refers 
> >> to not freeing memory that was allocated. I'm guessing you mean that 
> >> your program is overwriting memory that it shouldn't.
> >> 
> >> > the offending code snippet reads as follows:
> >> > unsigned char pad_len;
> >> > pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);
> >> > if(pad_len%30 == 0)
> >> > pad_len = 9;
> >> > if(packet->size > 0){
> >> > 	memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);
> >> > 	packet->size += pad_len;
> >> > 	packet->data.buf[0] = pad_len;
> >> > 	for(j=1; j< pad_len; j++)
> >> > 	packet->data.buf[j] = (char)256%j;
> >> > 	EnDeCrypt(packet->data.buf, packet->size, key, KEYLEN);
> >> > }
> >> > 
> >> > As the data.buf is statically allocated, valgrind does not complain
> >> > anything about it.
> >> > Is it running out of memory? or any other problem?
> >> > Any suggestion?
> >> 
> >> 
> >> There is no dynamic memory allocation in the fragment you have shown us, 
> >> so it can't be running out of memory.
> >> 
> >
> > You are absolutely right. I guess i'm misinformed about terminologies. Thanks
> >
> >> Are you sure that packet->size + pad_len < sizeof packet->data.buf? That 
> >> check should probably be in the code, unless there are constraints on 
> >> the packet size that guarantee that it is always true.
> >
> > Packet size can be as high as 230 with pad_len. pad_len is unsigned char. so it should cover upto 256. sizeof(packet->data.buf) is 8 so it should not be a problem. Though i guess should put a checking into the code anyway.
> 
> How long is your actual packet buffer? - you say above that it's 8 but
> you might be misunderstanding the question.  If not, then you'll have
> problems.
> 
Its given in the original post.Below the code snippet, the structure definition of packet is given there. Its about 8192 bytes long. more than enough to hold multiple copies. I was talking about the data type of packet->data.buf which unsigned char.

> If at any time packet_size + pad_len is longer than the size of
> packet->data.buf then you'll be scribbling on memory you don't own.
> 
> Can you show us the code that defines and sets packet->data.buf
> 
> As packet->data.buf must contain at least as many bytes as the maximum
> length of pad_len, can you tell us what RANSTOP and RANSTART are
> #define'd to?
> 

3 and 9

> It might be worth printing pad_len just before the memmove and seeing
> what the value is when it blows up.
> -- 
> Online waterways route planner            | http://canalplan.eu
> Plan trips, see photos, check facilities  | http://canalplan.org.uk

[toc] | [prev] | [next] | [standalone]


#15105

Fromjt@toerring.de (Jens Thoms Toerring)
Date2011-12-28 12:46 +0000
Message-ID<9m0hdiF7lbU1@mid.uni-berlin.de>
In reply to#15100
ArifulHossain tuhin <etothepowerpi@gmail.com> wrote:
> On Wednesday, December 28, 2011 4:36:37 PM UTC+6, Philip Lantz wrote:
> > Are you sure that packet->size + pad_len < sizeof packet->data.buf? That 
> > check should probably be in the code, unless there are constraints on 
> > the packet size that guarantee that it is always true.

> Packet size can be as high as 230 with pad_len.

You mean that 'packet->size + pad_len' is never larger than 230?
How can that be if, as you claim yourself below, 'pad_len' can be
as large as 255?

> unsigned char pad_len;
> pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);

> pad_len is unsigned char. so it should cover upto 256.

I guess you mean 255. And then don't cast but do

 pad_len = ( random( ) % RANSTOP + RANSTART ) & 0xFF;

Unsigned char must cover at least the range from 0 to 255 but it
can have a larger range (char does not necessarily just have 8
bits).

> if(pad_len%30 == 0)
> pad_len = 9;

This looks rather strange. Why reduce 'pad_len' to 9 only when
it's a multiple of 30?

> if(packet->size > 0){
>         memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);

So what exactly are 'pad_len' and 'packet->size' when it segfaults?
If it really crashes at this point then they must be something
different from what you expect them to be.

>         packet->size += pad_len;
>         packet->data.buf[0] = pad_len;
>         for(j=1; j< pad_len; j++)
>         packet->data.buf[j] = (char)256%j;

Casting 256 to a char looks rather ridiculous. Typiclally chars
are in the range from 0 to 255 or -128 to 127. What are you at-
tempting to achieve with all this?

>         EnDeCrypt(packet->data.buf, packet->size, key, KEYLEN);
> }
> 
> the packet data structure is decalared as follows:
> 
> struct rtp_packet {
>     size_t      size;
>     ................
>     (some fields)
>     ................
>     struct rtp_packet *next;
>     struct rtp_packet *prev;
>     union {
>         rtp_hdr_t       header;
>         unsigned char   buf[8192];
>     } data;

> sizeof(packet->data.buf) is 8 so it should not be a problem. Though
> i guess should put a checking into the code anyway.

The size of the buffer 'buf' must be 8192. Where did you get that
8 from?
                              Regards, Jens
-- 
  \   Jens Thoms Toerring  ___      jt@toerring.de
   \__________________________      http://toerring.de

[toc] | [prev] | [next] | [standalone]


#15115

FromArifulHossain tuhin <etothepowerpi@gmail.com>
Date2011-12-28 05:54 -0800
Message-ID<20569280.572.1325080494206.JavaMail.geo-discussion-forums@prez5>
In reply to#15077
Thank you for everyone's help. i have solved it. apparently i was destorying packet in place where i should not have.

[toc] | [prev] | [next] | [standalone]


#15117

FromBen Bacarisse <ben.usenet@bsb.me.uk>
Date2011-12-28 14:46 +0000
Message-ID<0.6f2a3d601d24dc231a9a.20111228144607GMT.8739c4eouo.fsf@bsb.me.uk>
In reply to#15077
ArifulHossain tuhin <etothepowerpi@gmail.com> writes:

> My application is leaking memory in a memmove call. I have made it sure from coredumps. 
>
> the offending code snippet reads as follows:
> unsigned char pad_len;
> pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);
> if(pad_len%30 == 0)
> pad_len = 9;
> if(packet->size > 0){
> 	memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);
> 	packet->size += pad_len;
> 	packet->data.buf[0] = pad_len;
> 	for(j=1; j< pad_len; j++)
> 	packet->data.buf[j] = (char)256%j;

Two small points, probably unrelated to your segfault.  Why start j at
1?  When pad_len > 0 you will leave data.buf[0] as it was, which looks
odd to a casual observer.  Secondly, the cast to char does nothing.  The
operands of % undergo the usual arithmetic conversions so (char)256 will
be promoted to int which is the type of the literal constant 256.

> 	EnDeCrypt(packet->data.buf, packet->size, key, KEYLEN);
> }

<snip>
-- 
Ben.

[toc] | [prev] | [next] | [standalone]


#15131

FromPhilip Lantz <prl@canterey.us>
Date2011-12-28 12:20 -0800
Message-ID<MPG.296519dbc2614653989681@news.eternal-september.org>
In reply to#15117
Ben Bacarisse wrote:
> ArifulHossain writes:
> > the offending code snippet reads as follows:
> > unsigned char pad_len;
> > pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);
> > if(pad_len%30 == 0)
> > pad_len = 9;
> > if(packet->size > 0){
> > 	memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);
> > 	packet->size += pad_len;
> > 	packet->data.buf[0] = pad_len;
> > 	for(j=1; j< pad_len; j++)
> > 	packet->data.buf[j] = (char)256%j;
> 
> Two small points, probably unrelated to your segfault.  Why start j at
> 1?  When pad_len > 0 you will leave data.buf[0] as it was, which looks
> odd to a casual observer.  Secondly, the cast to char does nothing.  The
> operands of % undergo the usual arithmetic conversions so (char)256 will
> be promoted to int which is the type of the literal constant 256.

    Interesting that you should make these two comments. I had written 
both of these comments in my earlier response, but then deleted them 
before sending. It turns out they are both wrong.

The loop starts j at 1 because data.buf[0] is set immediately before the 
loop.

Your comment on the (char) cast (which was exactly what I would have 
written) is true, but on any machine the OP is likely to be using, it 
will also convert the value 256 to 0. This means that every byte of the 
padding except the first is set to 0.

In the off chance that char is more than 8 bits, the padding is set to 
the sequence: 0 0 1 0 1 4 4 0 ...  It seems unlikely this is what is 
intended.

Philip

[toc] | [prev] | [next] | [standalone]


#15138

FromBen Bacarisse <ben.usenet@bsb.me.uk>
Date2011-12-28 21:50 +0000
Message-ID<0.d1faadd4b89dd085f51f.20111228215013GMT.87lipwcqne.fsf@bsb.me.uk>
In reply to#15131
Philip Lantz <prl@canterey.us> writes:

> Ben Bacarisse wrote:
>> ArifulHossain writes:
>> > the offending code snippet reads as follows:
>> > unsigned char pad_len;
>> > pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);
>> > if(pad_len%30 == 0)
>> > pad_len = 9;
>> > if(packet->size > 0){
>> > 	memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);
>> > 	packet->size += pad_len;
>> > 	packet->data.buf[0] = pad_len;
>> > 	for(j=1; j< pad_len; j++)
>> > 	packet->data.buf[j] = (char)256%j;
>> 
>> Two small points, probably unrelated to your segfault.  Why start j at
>> 1?  When pad_len > 0 you will leave data.buf[0] as it was, which looks
>> odd to a casual observer.  Secondly, the cast to char does nothing.  The
>> operands of % undergo the usual arithmetic conversions so (char)256 will
>> be promoted to int which is the type of the literal constant 256.
>
>     Interesting that you should make these two comments. I had written 
> both of these comments in my earlier response, but then deleted them 
> before sending. It turns out they are both wrong.

So they are.

> The loop starts j at 1 because data.buf[0] is set immediately before the 
> loop.

Ah, now that's me just not paying attention.

> Your comment on the (char) cast (which was exactly what I would have 
> written) is true, but on any machine the OP is likely to be using, it 
> will also convert the value 256 to 0. This means that every byte of the 
> padding except the first is set to 0.

How odd.  I never considered the value, thinking that it's just another
pointless cast.  In a way it probably is pointless but for other
reasons...

> In the off chance that char is more than 8 bits, the padding is set to 
> the sequence: 0 0 1 0 1 4 4 0 ...  It seems unlikely this is what is 
> intended.

No, I doubt it is.  Anyway, thanks for paying more attention that I did!

-- 
Ben.

[toc] | [prev] | [next] | [standalone]


#15125

FromBarry Schwarz <schwarzb@dqel.com>
Date2011-12-28 10:36 -0800
Message-ID<p0mmf718v78395o7h06ak3ut4krp83fnra@4ax.com>
In reply to#15077
On Wed, 28 Dec 2011 01:24:59 -0800 (PST), ArifulHossain tuhin
<etothepowerpi@gmail.com> wrote:

>My application is leaking memory in a memmove call. I have made it sure from coredumps. 
>
>the offending code snippet reads as follows:
>unsigned char pad_len;
>pad_len = (unsigned char)((random() % RANSTOP ) + RANSTART);

Are you using the cast to suppress an over-zealous diagnostic about
conversion between signed and unsigned or about int to char?  If not,
it serves no purpose.

In a subsequent message, you stated RANSTOP was 3 and RANSTART was 9.
random() is not a standard function but if it works like rand() than
the first addend will be in the range of 0 to 2.  Even if random()
produces negative values, the range would be -2 to 2.  Consequently
the sum must be in the range 9 to 11 (or 7 to 11).

>if(pad_len%30 == 0)

So when will this ever evaluate to true?

>pad_len = 9;

Even without braces, indenting is still a good idea.

>if(packet->size > 0){

What is the initial value of size?

>	memmove(packet->data.buf + pad_len, packet->data.buf, packet->size);

Is this the only call to memmove in your program?  If this is indeed
the statement causing the seg fault, you need to look at the current
values of pad_len and size.  Since pad_len <= 11, size is probably the
real culprit.

>	packet->size += pad_len;

If this block is in a loop that does not terminate as expected, it is
quite reasonable for size to grow larger than expected.

>	packet->data.buf[0] = pad_len;
>	for(j=1; j< pad_len; j++)
>	packet->data.buf[j] = (char)256%j;

This would not silence the diagnostic I mentioned above.  The division
is performed on int values so (char)256 is promoted.  The result is an
int which could generate either the signed->unsigned or int->char
complaints.

Changing it to (char)(256%j) could still lead to the signed->unsigned
diagnostic.

For the cast to have any value, it needs to be (unsigned char)(256%j).
Since the existing code apparently did not cause any of the
diagnostics which would prompt this, I have to assume this cast is
pointless also.

>	EnDeCrypt(packet->data.buf, packet->size, key, KEYLEN);

I don't know if it matters, but 256%j will generate multiple instances
of 0 in various elements of buf.

>}
>
>the packet data structure is decalared as follows:
>
>struct rtp_packet {
>    size_t      size;
>    ................
>    (some fields)
>    ................
>    struct rtp_packet *next;
>    struct rtp_packet *prev;
>    union {
>	rtp_hdr_t       header;
>	unsigned char   buf[8192];
>    } data;
>}; 
>
>As the data.buf is statically allocated, valgrind does not complain anything about it.
>Is it running out of memory? or any other problem?
>Any suggestion?

-- 
Remove del for email

[toc] | [prev] | [standalone]


Back to top | Article view | comp.lang.c


csiph-web