Skip to content
This repository was archived by the owner on Dec 8, 2024. It is now read-only.

Correctly handle potential popen() failures #63

Closed
wants to merge 3 commits into from
Closed

Correctly handle potential popen() failures #63

wants to merge 3 commits into from

Conversation

ikeydoherty
Copy link

I believe the commit descriptions are sufficient here :)
I had a look through the code earlier today, relevant discussion can be found on Google+: https://plus.google.com/u/0/+IkeyDoherty/posts/i7KeTu5hnRv

Realise it was quite possible you wouldn't see the post, so here are the relevant fixes.

Something I believe that does need addressing also however is your use of splitting strings.with your
str_replace function. Please note that every time you call it, you have a newly allocated string returned
from the function which you need to free using g_free().

Essentially, you have a whole bunch of strings being allocated and not freed. (Also look at your use of g_strdup within the method, that's two allocated and never-freed strings hanging around (The memory itself is never free'd)

If I have some time I might tackle this.

Hope this is somewhat helpful to you :)

Michael Ikey Doherty added 2 commits November 28, 2013 09:59
If we fail to run popen() to determine the last user for any reason,
be it I/O error or even memory allocation, we should immediately exit
the method, as we can no longer safely continue.
If we fail to run popen() to obtain the lsb description for any reason,
we should attempt to work around the situation (much like in the previous
commit).

This commit also switches the fixed length within the fgets call to
use sizeof, ensuring we always have the correct size as specified
in the declaration of our lsb_description array.
@ikeydoherty
Copy link
Author

In fact I'll have that commit up in a few minutes..

This commit also drops the redundant replacements in html_encode by
making use of g_markup_printf_escaped, which will safely escape
non displayable markup characters such as < > &, etc. using their
HTML-safe equivalents, such as &amp;.
@ikeydoherty
Copy link
Author

You may need to test that last commit on your machine just to be sure, but its essentially a lot better than the previous implementation.

I've tried to keep my commits within the same.. indent.. that you're using, though to be quite fair there
doesn't seem to be any style consistency. Perhaps a roadmap item? :)

@ikeydoherty
Copy link
Author

@clefebvre highlighting you here because I can't assign (obviously) :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants