Re: rpcrt4: Introduce a new function, safe_multiply, which will raise an exception if a multiply overflows a 4-byte integer.

[Alexandre Julliard]

Robert Shearman <rob(a)codeweavers.com> writes:
> +    unsigned char overflow;
> +#if defined(__i386__) && defined (__GNUC__)
> +    __asm__(
> +            "mull %3\n\t"
> +            "seto %%cl"
> +            : "=a" (ret), "=c" (overflow) /* outputs: eax, cl */
> +            : "%a" (a), "rm" (b) /* inputs: eax, any register or memory
> +                                  * address (params are interchangable) */
> +            : "edx" /* edx clobbered */);
> +#else
> +    ret = a * b;
> +    overflow = (b && (ret / b != a));
> +#endif

That asm seems really overkill. Something like:

  ULONGLONG ret = (ULONGLONG)a * b;
  if (ret > 0xffffffff)

will generate pretty much the same code with a recent gcc, and it's a
lot more readable.

BTW could you please add a sequence number in the subject line for
your patches?  It makes things a lot easier for me when the mails
arrive out of order. Thanks.

-- 
Alexandre Julliard
julliard(a)winehq.org

rpcrt4: Introduce a new function, safe_multiply, which will raise an exception if a multiply overflows a 4-byte integer.

[Robert Shearman]

Alexandre Julliard wrote:
> Robert Shearman <rob(a)codeweavers.com> writes:
> > +    unsigned char overflow;
> > +#if defined(__i386__) && defined (__GNUC__)
> > +    __asm__(
> > +            "mull %3\n\t"
> > +            "seto %%cl"
> > +            : "=a" (ret), "=c" (overflow) /* outputs: eax, cl */
> > +            : "%a" (a), "rm" (b) /* inputs: eax, any register or memory
> > +                                  * address (params are interchangable) */
> > +            : "edx" /* edx clobbered */);
> > +#else
> > +    ret = a * b;
> > +    overflow = (b && (ret / b != a));
> > +#endif
>
> That asm seems really overkill. Something like:
>
> ULONGLONG ret = (ULONGLONG)a * b;
>  if (ret > 0xffffffff)
>
> will generate pretty much the same code with a recent gcc, and it's a
> lot more readable.

Indeed it does. I'll resubmit the patch set with this change.

-- 
Rob Shearman