Re: CR: PR 143956: SDP has incorrect default track for 3gp Rel6 multirate conten

Tom Marshall wrote:
>> -2112,8 +2112,8 
>>
>>     memset((void*)pDefaultTracks, MAX_UINT32,
>>         ulNumStreamGroups*(sizeof(UINT32)));
>>-    memset((void*)pDefaultSet, MAX_UINT32,
>>-        ulNumStreamGroups*(sizeof(UINT32)));
>>+    memset((void*)pDefaultSet, FALSE,
>>+        ulNumStreamGroups*(sizeof(BOOL)));
> 
> 
> This is surely functional because FALSE is #define'd to
0, but it looks
> really strange to me.  The second parameter to memset
is an int representing
> a byte value (an old salty C-ism, of course) and it
looks strange to see a
> boolean-ish value there -- kinda like seeing NULL
assigned to an int type.
> 
> Could we use 0 instead of FALSE?
> 

Good point - I find the other assignment more concerning,

 >>     memset((void*)pDefaultTracks, MAX_UINT32,
 >>         ulNumStreamGroups*(sizeof(UINT32)));

Which presumably works because MAX_UINT32 truncates to char
FF so
it comes out the same in the end. How about this (tested on
win32-i386-vc7)

Index: sdpmdgen.cpp
============================================================
=======
RCS file: /cvsroot/protocol/sdp/sdpmdgen.cpp,v
retrieving revision 1.31.2.14
diff -u -w -r1.31.2.14 sdpmdgen.cpp
--- sdpmdgen.cpp        11 Jul 2005 23:11:04 -0000     
1.31.2.14
+++ sdpmdgen.cpp        11 Jul 2005 23:31:42 -0000
 -2110,9
+2110,12 
          return HXR_OUTOFMEMORY;
      }

-    memset((void*)pDefaultTracks, MAX_UINT32,
+    // Initialize all to MAX_UINT32
+    memset((void*)pDefaultTracks, 0xFF,
          ulNumStreamGroups*(sizeof(UINT32)));
-    memset((void*)pDefaultSet, FALSE,
+
+    // Initialize all to FALSE
+    memset((void*)pDefaultSet, 0,
          ulNumStreamGroups*(sizeof(BOOL)));

      for (i = 0; i < ulNumStreamGroups; i++)




_______________________________________________
Helix-server-dev mailing list
Helix-server-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinf
o/helix-server-dev

On Mon, Jul 11, 2005 at 04:33:58PM -0700, Jamie Gordon
wrote:
> Tom Marshall wrote:
> >> -2112,8 +2112,8 
> >>
> >>    memset((void*)pDefaultTracks, MAX_UINT32,
> >>        ulNumStreamGroups*(sizeof(UINT32)));
> >>-    memset((void*)pDefaultSet, MAX_UINT32,
> >>-        ulNumStreamGroups*(sizeof(UINT32)));
> >>+    memset((void*)pDefaultSet, FALSE,
> >>+        ulNumStreamGroups*(sizeof(BOOL)));
> >
> >
> >This is surely functional because FALSE is
#define'd to 0, but it looks
> >really strange to me.  The second parameter to
memset is an int
> >representing a byte value (an old salty C-ism, of
course) and it looks
> >strange to see a boolean-ish value there -- kinda
like seeing NULL
> >assigned to an int type.
> >
> >Could we use 0 instead of FALSE?
> >
> 
> Good point - I find the other assignment more
concerning,
> 
> >>     memset((void*)pDefaultTracks, MAX_UINT32,
> >>         ulNumStreamGroups*(sizeof(UINT32)));
> 
> Which presumably works because MAX_UINT32 truncates to
char FF so
> it comes out the same in the end. How about this
(tested on
> win32-i386-vc7)
> 
> Index: sdpmdgen.cpp
>
============================================================
=======
> RCS file: /cvsroot/protocol/sdp/sdpmdgen.cpp,v
> retrieving revision 1.31.2.14
> diff -u -w -r1.31.2.14 sdpmdgen.cpp
> --- sdpmdgen.cpp        11 Jul 2005 23:11:04 -0000     
1.31.2.14
> +++ sdpmdgen.cpp        11 Jul 2005 23:31:42 -0000
>  -2110,9 +2110,12 
>          return HXR_OUTOFMEMORY;
>      }
> 
> -    memset((void*)pDefaultTracks, MAX_UINT32,
> +    // Initialize all to MAX_UINT32
> +    memset((void*)pDefaultTracks, 0xFF,
>          ulNumStreamGroups*(sizeof(UINT32)));
> -    memset((void*)pDefaultSet, FALSE,
> +
> +    // Initialize all to FALSE
> +    memset((void*)pDefaultSet, 0,
>          ulNumStreamGroups*(sizeof(BOOL)));
> 
>      for (i = 0; i < ulNumStreamGroups; i++)

It's an improvement, I suppose... sorry, I didn't realize I
was opening a
can of worms with that comment.

-- 
An American is a man with two arms and four wheels.
        -- A Chinese child
_______________________________________________
Helix-server-dev mailing list
Helix-server-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinf
o/helix-server-dev

Tom Marshall wrote:
> On Mon, Jul 11, 2005 at 04:33:58PM -0700, Jamie Gordon
wrote:
>>Good point - I find the other assignment more
concerning,
>>
>>
>>>>    memset((void*)pDefaultTracks,
MAX_UINT32,
>>>>        ulNumStreamGroups*(sizeof(UINT32)));
>>
>>Which presumably works because MAX_UINT32 truncates
to char FF so
>>it comes out the same in the end. How about this
(tested on
>>win32-i386-vc7)


> It's an improvement, I suppose... sorry, I didn't
realize I was opening a
> can of worms with that comment.

Actually I just noticed that there is already a loop right
below that,
so we really may as well just initialize these arrays inside
that loop.
I'll send a new CR sometime today or tomorrow.

Thanks,
Jamie

_______________________________________________
Helix-server-dev mailing list
Helix-server-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinf
o/helix-server-dev