The Wayback Machine - https://web.archive.org/web/20201220211728/https://github.com/swaywm/sway/pull/3473
Skip to content

/ sway

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allocate minimum size necessary in pango text functions. #3473

Merged
merged 2 commits into from Jan 20, 2019
Merged

Allocate minimum size necessary in pango text functions. #3473

merged 2 commits into from Jan 20, 2019

Conversation

@cedws
Copy link
Contributor

@cedws cedws commented Jan 19, 2019 •
edited

Almost forgot about this. See here (annoyingly this doesn't link directly to the comment. It's in one of the resolved conversations...).
@emersion
emersion reviewed Jan 19, 2019
View changes
common/pango.c Outdated Show resolved Hide resolved
@cedws
cedws reviewed Jan 19, 2019
View changes
.gitignore Outdated Show resolved Hide resolved
@cedws
Copy link
Contributor Author

@cedws cedws commented Jan 19, 2019

Anomaly is causing CI build failure. Code compiles.

...
(1/5) Probing GDK-Pixbuf loader modules...
(2/5) Reloading system manager configuration...
(3/5) Reloading device manager configuration...
(4/5) Arming ConditionNeedsUpdate...
(5/5) Updating the MIME type database...
:: Searching databases for updates...
:: Searching AUR for updates...
Get https://aur.archlinux.org/rpc.php?arg%5B%5D=yay-bin&type=info&v=5: dial tcp 5.9.250.164:443: i/o timeout
@mstoeckl
mstoeckl reviewed Jan 19, 2019
View changes
include/util.h Outdated Show resolved Hide resolved
@cedws cedws force-pushed the cedws:feature/PangoAllocateSizeNecessary branch from bd67efc to 19c6bc5 Jan 19, 2019
@cedws cedws changed the title Allocate minimum size necessary but limit to MAX_CHARS in pango text functions. Jan 19, 2019
@cedws cedws force-pushed the cedws:feature/PangoAllocateSizeNecessary branch 2 times, most recently from 304dbba to 476e5af Jan 19, 2019
@emersion
Copy link
Member

@emersion emersion commented Jan 20, 2019

Seems like you added some .idea junk to your commit (please don't add it to .gitignore).
@cedws
Copy link
Contributor Author

@cedws cedws commented Jan 20, 2019

Yep, just force pushed.
@emersion
emersion reviewed Jan 20, 2019
View changes
include/util.h Outdated Show resolved Hide resolved
@cedws cedws force-pushed the cedws:feature/PangoAllocateSizeNecessary branch from 476e5af to 894c8df Jan 20, 2019
@cedws
Allocate minimum size necessary in pango text functions.
Loading status checks…
445d948
@cedws cedws force-pushed the cedws:feature/PangoAllocateSizeNecessary branch from 894c8df to 445d948 Jan 20, 2019
@emersion
emersion approved these changes Jan 20, 2019
View changes
@ddevault
ddevault reviewed Jan 20, 2019
View changes
common/pango.c Show resolved Hide resolved
@ddevault
ddevault reviewed Jan 20, 2019
View changes
common/pango.c Show resolved Hide resolved
@cedws
Handle malloc failure.
Loading status checks…
83c543e
@ddevault
ddevault approved these changes Jan 20, 2019
View changes
@emersion
emersion reviewed Jan 20, 2019
View changes
common/pango.c
strcpy(&buf[sizeof(buf) - sizeof(overflow)], overflow);
}
// Add one since vsnprintf excludes null terminator.
int length = vsnprintf(NULL, 0, fmt, args) + 1;

This comment has been minimized.

Sign in to view
@emersion

emersion Jan 20, 2019
Member

Nitpick: I prefer to use "length" (or "len") to mean the return value of strlen (aka without the terminating NULL byte) and "size" to mean the value passed to malloc et al (aka including the terminating NULL byte).

This comment has been minimized.

Sign in to view
@cedws

cedws Jan 20, 2019
Author Contributor

Yep, that's fair. I can change it if you really want.
@cedws
Copy link
Contributor Author

@cedws cedws commented Jan 20, 2019

In what cases do allocation failures need to be handled? Is it always? If that's the case, is that something that wasn't enforced in the past but is now?
@ddevault
Copy link
Member

@ddevault ddevault commented Jan 20, 2019

Just where it's important. In this case it's possibly exploitable by an evil client which sets its title to something ridiculous that we can't draw.
@emersion
Copy link
Member

@emersion emersion commented Jan 20, 2019

In a perfect world, the compositor would survive even if the device is running low on memory.
@emersion emersion merged commit 9df3a91 into swaywm:master Jan 20, 2019
1 check passed
1 check passed
builds.sr.ht: .build.yml builds.sr.ht job completed successfully
Details
@emersion
Copy link
Member

@emersion emersion commented Jan 20, 2019

Thanks!
grahnen added a commit to grahnen/sway that referenced this pull request Jan 23, 2019
@cedws @grahnen
Allocate minimum size necessary in pango text functions. (swaywm#3473)
98bf43d 
* Allocate minimum size necessary in pango text functions.

* Handle malloc failure.
@cedws cedws deleted the cedws:feature/PangoAllocateSizeNecessary branch Feb 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants
@cedws @emersion @ddevault @martinetd @mstoeckl
You can’t perform that action at this time.