Re: [PATCH v2 1/3] storage.dll16: Fix negation in IStream::Seek.

[Henri Verbeet]

On 11 February 2017 at 23:54, Zebediah Figura <z.figura12(a)gmail.com> wrote:
> @@ -1196,9 +1196,8 @@ HRESULT CDECL IStream16_fnSeek(IStream16 *iface, LARGE_INTEGER offset, DWORD whe
>                 break;
>         case SEEK_CUR:
>                 if (offset.u.HighPart < 0) {
> -                       /* FIXME: is this negation correct ? */
> -                       offset.u.HighPart = -offset.u.HighPart;
> -                       offset.u.LowPart = (0xffffffff ^ offset.u.LowPart)+1;
> +                       offset.u.HighPart = ~offset.u.HighPart;
> +                       offset.u.LowPart = -offset.u.LowPart;

I don't think that's quite correct either. Consider e.g.
0xffffffff00000000, the negation of which would be 0x0000000100000000.

[PATCH v2 1/3] storage.dll16: Fix negation in IStream::Seek.

[Zebediah Figura]

On 02/12/2017 10:12 AM, Henri Verbeet wrote:
> On 11 February 2017 at 23:54, Zebediah Figura <z.figura12(a)gmail.com> wrote:
> > @@ -1196,9 +1196,8 @@ HRESULT CDECL IStream16_fnSeek(IStream16 *iface, LARGE_INTEGER offset, DWORD whe
> >                 break;
> >         case SEEK_CUR:
> >                 if (offset.u.HighPart < 0) {
> > -                       /* FIXME: is this negation correct ? */
> > -                       offset.u.HighPart = -offset.u.HighPart;
> > -                       offset.u.LowPart = (0xffffffff ^ offset.u.LowPart)+1;
> > +                       offset.u.HighPart = ~offset.u.HighPart;
> > +                       offset.u.LowPart = -offset.u.LowPart;
>
> I don't think that's quite correct either. Consider e.g.
> 0xffffffff00000000, the negation of which would be 0x0000000100000000.

Whoops, you're right. Thanks for catching that.