Closed Bug 569943 Opened 10 years ago Closed 10 years ago

Need to explicitly check Enable/Disable arguments

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: vlad, Assigned: bjacob)

Details

Attachments

(2 files)

We need to explicitly check for valid arguments to Enable/Disable, instead of just passing them down to GL.
Comment on attachment 449705 [details] [diff] [review]
Check Enable/Disable/IsEnabled enums

Wow, are those really the only enable caps that are valid?  Thought there would be a lot more, crazy.

r+, with the following style changes:

>+NS_IMETHODIMP WebGLContext::Enable(WebGLuint cap)
>+{
>+    if (ValidateCapabilityEnum(cap)) {
>+        MakeContextCurrent();
>+        gl->fEnable(cap);
>+        return NS_OK;
>+    } else {
>+        return ErrorInvalidEnum("Enable: invalid capability enum");
>+    }
>+}

Treat if clauses as "exceptional" cases, and have the normal flow be the "good" path for readability, i.e.:

{
    if (!ValidateCapabilityEnum(cap))
        return ErrorInvalidEnum(...);

    MakeContextCurrent();
    fEnable(...);
    return NS_OK;
}

>+NS_IMETHODIMP WebGLContext::Disable(WebGLuint cap)
>+{
>+    if (ValidateCapabilityEnum(cap)) {
>+        MakeContextCurrent();
>+        gl->fDisable(cap);
>+        return NS_OK;
>+    } else {
>+        return ErrorInvalidEnum("Disable: invalid capability enum");
>+    }
>+}

Same.

> NS_IMETHODIMP
>-WebGLContext::IsEnabled(WebGLenum k, WebGLboolean *retval)
>+WebGLContext::IsEnabled(WebGLenum cap, WebGLboolean *retval)
> {
>-    MakeContextCurrent();
>-    *retval = gl->fIsEnabled(k);
>-    return NS_OK;
>+    if(ValidateCapabilityEnum(cap)) {
>+        MakeContextCurrent();
>+        *retval = gl->fIsEnabled(cap);
>+        return NS_OK;
>+    } else {
>+        *retval = 0; // as per the OpenGL ES spec
>+        return ErrorInvalidEnum("IsEnabled: invalid capability enum");
>+    }

Same.
Attachment #449705 - Flags: review?(vladimir) → review+
Attachment #449713 - Flags: review?(vladimir) → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.