Skip to content

Commit

Permalink
src: refactor and apply fixes
Browse files Browse the repository at this point in the history
- Defer the initialization of the `op` variable to the `default` switch case to avoid a compiler warning.
- Use a `default` switch case with a null statement if some enum values aren't suppsed to be handled, this avoids a compiler warning.
- Migrate from librsvg's deprecated `rsvg_handle_get_dimensions()` and `rsvg_handle_render_cairo()` functions to the new `rsvg_handle_get_intrinsic_size_in_pixels()` and `rsvg_handle_render_document()` respectively.
- Avoid calling virtual methods in constructors/destructors to avoid bypassing virtual dispatch.
- Remove unused private field `backend` in the `Backend` class.
- Fix a case of use-after-free.
- Fix usage of garbage value by filling the allocated memory entirely with zeros if it's not modified.
- Fix a potential memory leak.
  • Loading branch information
VoltrexKeyva authored and zbjornson committed Apr 17, 2023
1 parent 38e0a32 commit f3184ba
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 15 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,16 @@ project adheres to [Semantic Versioning](http://semver.org/).
(Unreleased)
==================
### Changed
* Defer the initialization of the `op` variable to the `default` switch case to avoid a compiler warning. (#2229)
* Use a `default` switch case with a null statement if some enum values aren't suppsed to be handled, this avoids a compiler warning. (#2229)
* Migrate from librsvg's deprecated `rsvg_handle_get_dimensions()` and `rsvg_handle_render_cairo()` functions to the new `rsvg_handle_get_intrinsic_size_in_pixels()` and `rsvg_handle_render_document()` respectively. (#2229)
* Avoid calling virtual methods in constructors/destructors to avoid bypassing virtual dispatch. (#2229)
* Remove unused private field `backend` in the `Backend` class. (#2229)
### Added
### Fixed
* Fix a case of use-after-free. (#2229)
* Fix usage of garbage value by filling the allocated memory entirely with zeros if it's not modified. (#2229)
* Fix a potential memory leak. (#2229)

2.11.2
==================
Expand Down
3 changes: 2 additions & 1 deletion src/Canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,9 @@ NAN_METHOD(Canvas::New) {
}

if (!backend->isSurfaceValid()) {
const char *error = backend->getError();
delete backend;
return Nan::ThrowError(backend->getError());
return Nan::ThrowError(error);
}

Canvas* canvas = new Canvas(backend);
Expand Down
15 changes: 12 additions & 3 deletions src/CanvasRenderingContext2d.cc
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,8 @@ Context2d::blur(cairo_surface_t *surface, int radius) {
// get width, height
int width = cairo_image_surface_get_width( surface );
int height = cairo_image_surface_get_height( surface );
unsigned* precalc =
(unsigned*)malloc(width*height*sizeof(unsigned));
const unsigned int size = width * height * sizeof(unsigned);
unsigned* precalc = (unsigned*)malloc(size);
cairo_surface_flush( surface );
unsigned char* src = cairo_image_surface_get_data( surface );
double mul=1.f/((radius*2)*(radius*2));
Expand All @@ -627,6 +627,8 @@ Context2d::blur(cairo_surface_t *surface, int radius) {
unsigned char* pix = src;
unsigned* pre = precalc;

bool modified = false;

pix += channel;
for (y=0;y<height;y++) {
for (x=0;x<width;x++) {
Expand All @@ -635,10 +637,15 @@ Context2d::blur(cairo_surface_t *surface, int radius) {
if (y>0) tot+=pre[-width];
if (x>0 && y>0) tot-=pre[-width-1];
*pre++=tot;
if (!modified) modified = true;
pix += 4;
}
}

if (!modified) {
memset(precalc, 0, size);
}

// blur step.
pix = src + (int)radius * width * 4 + (int)radius * 4 + channel;
for (y=radius;y<height-radius;y++) {
Expand Down Expand Up @@ -1432,7 +1439,7 @@ NAN_GETTER(Context2d::GetGlobalCompositeOperation) {
Context2d *context = Nan::ObjectWrap::Unwrap<Context2d>(info.This());
cairo_t *ctx = context->context();

const char *op = "source-over";
const char *op{};
switch (cairo_get_operator(ctx)) {
// composite modes:
case CAIRO_OPERATOR_CLEAR: op = "clear"; break;
Expand Down Expand Up @@ -1469,6 +1476,7 @@ NAN_GETTER(Context2d::GetGlobalCompositeOperation) {
case CAIRO_OPERATOR_HSL_LUMINOSITY: op = "luminosity"; break;
// non-standard:
case CAIRO_OPERATOR_SATURATE: op = "saturate"; break;
default: op = "source-over";
}

info.GetReturnValue().Set(Nan::New(op).ToLocalChecked());
Expand Down Expand Up @@ -2507,6 +2515,7 @@ Context2d::setTextPath(double x, double y) {
pango_layout_get_pixel_extents(_layout, NULL, &logical_rect);
x -= logical_rect.width;
break;
default: ;
}

y -= getBaselineAdjustment(_layout, state->textBaseline);
Expand Down
18 changes: 13 additions & 5 deletions src/Image.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1192,11 +1192,13 @@ Image::loadSVGFromBuffer(uint8_t *buf, unsigned len) {
return CAIRO_STATUS_READ_ERROR;
}

RsvgDimensionData *dims = new RsvgDimensionData();
rsvg_handle_get_dimensions(_rsvg, dims);
double d_width;
double d_height;

width = naturalWidth = dims->width;
height = naturalHeight = dims->height;
rsvg_handle_get_intrinsic_size_in_pixels(_rsvg, &d_width, &d_height);

width = naturalWidth = d_width;
height = naturalHeight = d_height;

status = renderSVGToSurface();
if (status != CAIRO_STATUS_SUCCESS) {
Expand Down Expand Up @@ -1232,7 +1234,13 @@ Image::renderSVGToSurface() {
return status;
}

gboolean render_ok = rsvg_handle_render_cairo(_rsvg, cr);
RsvgRectangle viewport = {
.x = 0,
.y = 0,
.width = static_cast<double>(width),
.height = static_cast<double>(height),
};
gboolean render_ok = rsvg_handle_render_document(_rsvg, cr, &viewport, nullptr);
if (!render_ok) {
g_object_unref(_rsvg);
cairo_destroy(cr);
Expand Down
5 changes: 2 additions & 3 deletions src/backend/Backend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Backend::Backend(std::string name, int width, int height)

Backend::~Backend()
{
this->destroySurface();
Backend::destroySurface();
}

void Backend::init(const Nan::FunctionCallbackInfo<v8::Value> &info) {
Expand Down Expand Up @@ -97,8 +97,7 @@ bool Backend::isSurfaceValid(){

BackendOperationNotAvailable::BackendOperationNotAvailable(Backend* backend,
std::string operation_name)
: backend(backend)
, operation_name(operation_name)
: operation_name(operation_name)
{
msg = "operation " + operation_name +
" not supported by backend " + backend->getName();
Expand Down
1 change: 0 additions & 1 deletion src/backend/Backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ class Backend : public Nan::ObjectWrap
class BackendOperationNotAvailable: public std::exception
{
private:
Backend* backend;
std::string operation_name;
std::string msg;

Expand Down
2 changes: 1 addition & 1 deletion src/backend/PdfBackend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ using namespace v8;

PdfBackend::PdfBackend(int width, int height)
: Backend("pdf", width, height) {
createSurface();
PdfBackend::createSurface();
}

PdfBackend::~PdfBackend() {
Expand Down
2 changes: 1 addition & 1 deletion src/backend/SvgBackend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ using namespace v8;

SvgBackend::SvgBackend(int width, int height)
: Backend("svg", width, height) {
createSurface();
SvgBackend::createSurface();
}

SvgBackend::~SvgBackend() {
Expand Down
1 change: 1 addition & 0 deletions src/register_font.cc
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ get_pango_font_description(unsigned char* filepath) {
}

pango_font_description_set_family_static(desc, family);
free(family);
pango_font_description_set_weight(desc, get_pango_weight(table->usWeightClass));
pango_font_description_set_stretch(desc, get_pango_stretch(table->usWidthClass));
pango_font_description_set_style(desc, get_pango_style(face->style_flags));
Expand Down

0 comments on commit f3184ba

Please sign in to comment.