Closed Bug 901947 Opened 6 years ago Closed 6 years ago

don't null-check |new X| in content/

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(3 files, 1 obsolete file)

We have infallible new now.
Caught by grepping for NS_ERROR_OUT_OF_MEMORY and seeing if the checked thing
was allocated via |new|.

There are still a fair number of places where we throw NS_ERROR_OUT_OF_MEMORY.
Most of them are dealing with arrays, and I didn't want to check whether the
arrays in question were infallible or not.  Others are from various ::Create-type
functions and I wasn't sure those work identically to |new|.
Attachment #786289 - Flags: review?(bugs)
Assignee: nobody → nfroyd
Comment on attachment 786289 [details] [diff] [review]
don't null-check |new X| in content/

Review of attachment 786289 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/HTMLFrameSetElement.cpp
@@ +239,5 @@
>      count++;
>      commaX = spec.FindChar(sComma, commaX + 1);
>    }
>  
>    nsFramesetSpec* specs = new nsFramesetSpec[count];

Should this be fallible?
Comment on attachment 786289 [details] [diff] [review]
don't null-check |new X| in content/

I don't understand why we have (std::nothrow) with new. Remove it perhaps?

And yeah, could you make nsFramesetSpec* specs = new nsFramesetSpec[count]
use non-fallible new
Attachment #786289 - Flags: review?(bugs) → review+
Blocks: 899309
Carrying over r+.
Attachment #786289 - Attachment is obsolete: true
Attachment #787675 - Flags: review+
Comment on attachment 787673 [details] [diff] [review]
part 0a - use fallible_t instead of std::nothrow in CanvasRenderingContext2D.cpp

># HG changeset patch
># User Nathan Froyd <froydnj@gmail.com>
>
>part 0a - use fallible_t instead of std::nothrow in CanvasRenderingContext2D.cpp
>
>As requested.
>
>diff --git a/content/canvas/src/CanvasRenderingContext2D.cpp b/content/canvas/src/CanvasRenderingContext2D.cpp
>index fe50689..8fe6c63 100644
>--- a/content/canvas/src/CanvasRenderingContext2D.cpp
>+++ b/content/canvas/src/CanvasRenderingContext2D.cpp
>@@ -1036,31 +1036,32 @@ CanvasRenderingContext2D::GetInputStream(const char *aMimeType,
>   nsRefPtr<gfxASurface> surface;
> 
>   if (NS_FAILED(GetThebesSurface(getter_AddRefs(surface)))) {
>     return NS_ERROR_FAILURE;
>   }
> 
>   nsresult rv;
>   const char encoderPrefix[] = "@mozilla.org/image/encoder;2?type=";
>-  nsAutoArrayPtr<char> conid(new (std::nothrow) char[strlen(encoderPrefix) + strlen(aMimeType) + 1]);
>+  static const fallible_t fallible = fallible_t();
>+  nsAutoArrayPtr<char> conid(new (fallible) char[strlen(encoderPrefix) + strlen(aMimeType) + 1]);
> 
>   if (!conid) {
>     return NS_ERROR_OUT_OF_MEMORY;
>   }
> 
>   strcpy(conid, encoderPrefix);
>   strcat(conid, aMimeType);
> 
>   nsCOMPtr<imgIEncoder> encoder = do_CreateInstance(conid);
>   if (!encoder) {
>     return NS_ERROR_FAILURE;
>   }
> 
>-  nsAutoArrayPtr<uint8_t> imageBuffer(new (std::nothrow) uint8_t[mWidth * mHeight * 4]);
>+  nsAutoArrayPtr<uint8_t> imageBuffer(new (fallible) uint8_t[mWidth * mHeight * 4]);
>   if (!imageBuffer) {
>     return NS_ERROR_OUT_OF_MEMORY;
>   }
> 
>   nsRefPtr<gfxImageSurface> imgsurf =
>     new gfxImageSurface(imageBuffer.get(),
>                         gfxIntSize(mWidth, mHeight),
>                         mWidth * 4,
Attachment #787673 - Flags: review?(bugs) → review+
Comment on attachment 787674 [details] [diff] [review]
part 0b - use fallible allocation in HTMLFrameSetElement.cpp

>@@ -243,17 +245,18 @@ HTMLFrameSetElement::ParseRowCol(const nsAString & aValue,
>   PR_STATIC_ASSERT(NS_MAX_FRAMESET_SPEC_COUNT * sizeof(nsFramesetSpec) < (1 << 30));
>   int32_t commaX = spec.FindChar(sComma);
>   int32_t count = 1;
>   while (commaX != kNotFound && count < NS_MAX_FRAMESET_SPEC_COUNT) {
>     count++;
>     commaX = spec.FindChar(sComma, commaX + 1);
>   }
> 
>-  nsFramesetSpec* specs = new nsFramesetSpec[count];
>+  static const fallible_t fallible = fallible_t();
>+  nsFramesetSpec* specs = new (fallible) nsFramesetSpec[count];
I think we need only this case, since web page can control the 'count'
Attachment #787674 - Flags: review?(bugs) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.