5:29 a.m.
Hi guys!
One thing I noticed while browsing around Inkscape source, was the pattern
"allocate temp gtk string, call some api, deallocate temp string".
I think it makes sense to make a small "TempGString" class that captures
this (RAII) pattern, and deallocates the string on going out of scope. This
would have some advantages compared to current approach:
1) string would be freed regardless of how the code exits the block
(exceptions e.g.)
2) 2 lines instead of 3
3) easier on the eyes
Sample before-after code snippet:
Before:
if (!qname_prefix(code).id()) {
gchar *svg_name = g_strconcat(prefix, ":", g_quark_to_string(code),
NULL);
repr->setCodeUnsafe(g_quark_from_string(svg_name));
g_free(svg_name);
}
After:
if (!qname_prefix(code).id()) {
TempGString svg_name(g_strconcat(prefix, ":", g_quark_to_string(code),
NULL));
repr->setCodeUnsafe(g_quark_from_string(svg_name));
}
Syntax highlighted version available here: http://pastebin.com/t7p0TVfj
The constructor takes the freshly allocated string, and the destructor
calls g_free on it. There is an operator overload so that contexts that
expect ordinary pointers to gchar* gets what it want. This automagic
casting is a point I'm a bit ambivalent about myself; I wouldn't mind
having an accessor function "get_gchar_ptr()" or similar just to be super
explicit, thoughts on that?
I don't think this would break anything, however it might be that I'm
ignorant to some weird behaviour of ghar* or glib/gtk?
On a convention/code style notion, I looked through the code for RAII
(worst abbreviation for something so clear/simple semantically) and noticed
it's used in a couple other places, however I've still been wary of
introducing this refactoring to the code base as I don't know if you guys
would like it.
The pattern occurs on at least 140 places in the code base.
Cheers,
/Olof
9:38 p.m.
New subject: TempGString to simplify common pattern in code..?
Does anyone want to voice an opinion on this one or should I just go ahead
and do it? What about using implicit casting, you OK with that?
I think reviewing it should be fairly straight-forward as it's almost a
regex search-replace operation (just almost heh).
BTW I noticed there is a paste option on the web site too, so here it is:
https://inkscape.org/en/paste/9559/
Mvh
/Olof
-----------------
Är du systemutvecklare?
Spana in https://cilamp.se
On 3 May 2016 at 22:29, Olof Bjarnason <olof.bjarnason@...400...> wrote:
Hi guys!
One thing I noticed while browsing around Inkscape source, was the pattern
"allocate temp gtk string, call some api, deallocate temp string".
I think it makes sense to make a small "TempGString" class that captures
this (RAII) pattern, and deallocates the string on going out of scope. This
would have some advantages compared to current approach:
1) string would be freed regardless of how the code exits the block
(exceptions e.g.)
2) 2 lines instead of 3
3) easier on the eyes
Sample before-after code snippet:
Before:
if (!qname_prefix(code).id()) {
gchar *svg_name = g_strconcat(prefix, ":", g_quark_to_string(code),
NULL);
repr->setCodeUnsafe(g_quark_from_string(svg_name));
g_free(svg_name);
}
After:
if (!qname_prefix(code).id()) {
TempGString svg_name(g_strconcat(prefix, ":", g_quark_to_string(code),
NULL));
repr->setCodeUnsafe(g_quark_from_string(svg_name));
}
Syntax highlighted version available here: http://pastebin.com/t7p0TVfj
The constructor takes the freshly allocated string, and the destructor
calls g_free on it. There is an operator overload so that contexts that
expect ordinary pointers to gchar* gets what it want. This automagic
casting is a point I'm a bit ambivalent about myself; I wouldn't mind
having an accessor function "get_gchar_ptr()" or similar just to be super
explicit, thoughts on that?
I don't think this would break anything, however it might be that I'm
ignorant to some weird behaviour of ghar* or glib/gtk?
On a convention/code style notion, I looked through the code for RAII
(worst abbreviation for something so clear/simple semantically) and noticed
it's used in a couple other places, however I've still been wary of
introducing this refactoring to the code base as I don't know if you guys
would like it.
The pattern occurs on at least 140 places in the code base.
Cheers,
/Olof
10:40 p.m.
New subject: TempGString to simplify common pattern in code..?
This sounds like something that should already be covered by gtkmm or
even std::string. Usually it's better to look first to existing
solutions before inventing our own wheel.
IIRC Glib::ustring should cover the cases where we have content that is
guaranteed to be UTF-8. (Keep in mind that gtk+ apps have to keep track
of three different encodings and not get them mixed up.
For strings that are potentially different encodings we can use
std::string.
For content that looked like a string but is actually a byte sequence,
we can use std::vector<char> or std::vector<uint8_t>
On Fri, May 6, 2016, at 05:38 AM, Olof Bjarnason wrote:
Does anyone want to voice an opinion on this one or should I just go
ahead and do it? What about using implicit casting, you OK with that?
I think reviewing it should be fairly straight-forward as it's almost
a regex search-replace operation (just almost heh).
BTW I noticed there is a paste option on the web site too, so
here it is:
https://inkscape.org/en/paste/9559/
--
Jon A. Cruz
jon@...18...
10:51 p.m.
New subject: TempGString to simplify common pattern in code..?
Yeah, of course if there already exists a solution / pattern then that
would be preferrable...
(I have only looked at cases for deallocating gchar* using g_free, I do not
know what encoding is used inside the strings, and a TempGString object
would be agnostic to the encoding anyway.)
Anyone know if there is a RAII type for string handling in GLib? My google
fu fails me on this atm.
Mvh
/Olof
-----------------
Är du systemutvecklare?
Spana in https://cilamp.se
On 6 May 2016 at 15:40, Jon A. Cruz <jon@...18...> wrote:
This sounds like something that should already be covered by gtkmm or
even
std::string. Usually it's better to look first to existing solutions before
inventing our own wheel.
IIRC Glib::ustring should cover the cases where we have content that is
guaranteed to be UTF-8. (Keep in mind that gtk+ apps have to keep track of
three different encodings and not get them mixed up.
For strings that are potentially different encodings we can use
std::string.
For content that looked like a string but is actually a byte sequence, we
can use std::vector<char> or std::vector<uint8_t>
On Fri, May 6, 2016, at 05:38 AM, Olof Bjarnason wrote:
Does anyone want to voice an opinion on this one or should I just go ahead
and do it? What about using implicit casting, you OK with that?
I think reviewing it should be fairly straight-forward as it's almost a
regex search-replace operation (just almost heh).
BTW I noticed there is a paste option on the web site too, so here it is:
https://inkscape.org/en/paste/9559/
--
Jon A. Cruz
jon@...18...
------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications
Manager
Applications Manager provides deep performance insights into multiple
tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Inkscape-devel mailing list
Inkscape-devel(a)lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/inkscape-devel
11:54 p.m.
New subject: TempGString to simplify common pattern in code..?
OK, I read up a little on GLib and GTK+.
Both are (pure) C libraries, so I fail to see how they could provide a RAII
idiom? Last time I checked, C didn't have constructors/destructors...? Or
maybe there is some pre-processor magic to make it happen...?
std::string is nice (I would prefer it in fact) but it wouldn't call g_free
automatically when the gchar* variable goes out of scope.
Mvh
/Olof
-----------------
Är du systemutvecklare?
Spana in https://cilamp.se
On 6 May 2016 at 15:51, Olof Bjarnason <olof.bjarnason@...400...> wrote:
Yeah, of course if there already exists a solution / pattern then
that
would be preferrable...
(I have only looked at cases for deallocating gchar* using g_free, I do
not know what encoding is used inside the strings, and a TempGString object
would be agnostic to the encoding anyway.)
Anyone know if there is a RAII type for string handling in GLib? My google
fu fails me on this atm.
Mvh
/Olof
-----------------
Är du systemutvecklare?
Spana in https://cilamp.se
On 6 May 2016 at 15:40, Jon A. Cruz <jon@...18...> wrote:
> This sounds like something that should already be covered by gtkmm or
> even std::string. Usually it's better to look first to existing solutions
> before inventing our own wheel.
>
> IIRC Glib::ustring should cover the cases where we have content that is
> guaranteed to be UTF-8. (Keep in mind that gtk+ apps have to keep track of
> three different encodings and not get them mixed up.
>
> For strings that are potentially different encodings we can use
> std::string.
>
> For content that looked like a string but is actually a byte sequence, we
> can use std::vector<char> or std::vector<uint8_t>
>
>
> On Fri, May 6, 2016, at 05:38 AM, Olof Bjarnason wrote:
>
> Does anyone want to voice an opinion on this one or should I just go
> ahead and do it? What about using implicit casting, you OK with that?
>
> I think reviewing it should be fairly straight-forward as it's almost a
> regex search-replace operation (just almost heh).
>
> BTW I noticed there is a paste option on the web site too, so here it is:
>
> https://inkscape.org/en/paste/9559/
>
> --
> Jon A. Cruz
> jon@...18...
>
>
>
> ------------------------------------------------------------------------------
> Find and fix application performance issues faster with Applications
> Manager
> Applications Manager provides deep performance insights into multiple
> tiers of
> your business applications. It resolves application problems quickly and
> reduces your MTTR. Get your free trial!
> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
> _______________________________________________
> Inkscape-devel mailing list
> Inkscape-devel(a)lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/inkscape-devel
>
>
2517
days inactive
2520
days old
inkscape-devel@lists.inkscape.org
4 comments
2 participants
participants (2)
-
Jon A. Cruz -
Olof Bjarnason