[Webkit-unassigned] [Bug 22807] Cygwin/X WebKit/Gtk port

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=22807


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25951|review?                     |review-
               Flag|                            |




------- Comment #11 from darin at apple.com  2009-01-02 11:07 PDT -------
(From update of attachment 25951)
Patch needs a ChangeLog.

We'll need more rationale for this change.

Throughout WebCore and JavaScriptCore, we use int and unsigned to mean 32 bit
integers. We are not moving to a design where we use the <stdint.h> types.

Further, both WebCore and JavaScriptCore have been compiled with gcc and with
Microsoft's C compiler.

So I don't understand what's driving these changes? Are these fixing warnings
with a particular compiler? What version?

> -            concatenate(asString(v1)->value().rep(), JSImmediate::getTruncatedInt32(v2)) :
> +            concatenate(asString(v1)->value().rep(), static_cast<int>(JSImmediate::getTruncatedInt32(v2))) :

This change seems OK, but I'd like to understand why you see this warning and
no one else does.

> -        unsigned i;
> +        uint32_t i;
>  
>          bool isUInt32 = JSImmediate::getUInt32(subscript, i);

This change seems good (and the others like it), with no argument against it.
But I think it points out the trouble we get into when we start using uint32_t
in a few places. I think a better change to be consistent across the project
would be using unsigned rather than uint32_t in the definition of the getUInt32
function.

But again, I'm puzzled about why you're seeing this an no one else is.

> -                    int from = max(0, range.from() - static_cast<int>(start));
> -                    int to = 1 + min(range.to() - static_cast<int>(start), static_cast<int>(GlyphPage::size) - 1);
> +                    int from = max(0, static_cast<int>(range.from() - start));
> +                    int to = 1 + min(static_cast<int>(range.to() - start), static_cast<int>(GlyphPage::size) - 1);

I don't understand this change. Is there really a compiler that warns when you
subtract an unsigned from an int and then use that as an int? What compiler?
Maybe the issue is simply that you need to synchronize the list of ignored
warnings with the other Windows port makefiles?

On the other hand, the change seems harmless so we could do this one without
pondering further.

>  typedef NPObject* (*NPN_CreateObjectProcPtr) (NPP, NPClass *aClass);
>  typedef NPObject* (*NPN_RetainObjectProcPtr) (NPObject *obj);
>  typedef void (*NPN_ReleaseObjectProcPtr) (NPObject *obj);
> -typedef bool (*NPN_InvokeProcPtr) (NPP npp, NPObject *obj, NPIdentifier methodName, const NPVariant *args, unsigned argCount, NPVariant *result);
> -typedef bool (*NPN_InvokeDefaultProcPtr) (NPP npp, NPObject *obj, const NPVariant *args, unsigned argCount, NPVariant *result);
> +typedef bool (*NPN_InvokeProcPtr) (NPP npp, NPObject *obj, NPIdentifier methodName, const NPVariant *args, uint32_t argCount, NPVariant *result);
> +typedef bool (*NPN_InvokeDefaultProcPtr) (NPP npp, NPObject *obj, const NPVariant *args, uint32_t argCount, NPVariant *result);
>  typedef bool (*NPN_EvaluateProcPtr) (NPP npp, NPObject *obj, NPString *script, NPVariant *result);
>  typedef bool (*NPN_GetPropertyProcPtr) (NPP npp, NPObject *obj, NPIdentifier  propertyName, NPVariant *result);
>  typedef bool (*NPN_SetPropertyProcPtr) (NPP npp, NPObject *obj, NPIdentifier  propertyName, const NPVariant *value);

This is a public header, shared with other projects such as Mozilla, so
changing this is inappropriate unless it absolutely must be done. Please don't
make this change.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.