On Mon, 04 Mar 2013 22:30:40 -0000 Colomban Wendling git-noreply@geany.org wrote:
Log Message:
Scope: Fix mismatched allocator/deallocator
These are not mismatched, I'm using strdup() instead of strdup() for strings that must never be NULL. Unless we have a policy to always use g_strdup(), or there is a real g_/strdup vs. g_/free mismatch somewhere, I'm going to revert them.
Personally I prefer offensive programming: if something goes wrong, let it crash and burn, get a proper backtrace and fix it.
Le 07/03/2013 20:06, Dimitar Zhekov a écrit :
On Mon, 04 Mar 2013 22:30:40 -0000 Colomban Wendling git-noreply@geany.org wrote:
Log Message:
Scope: Fix mismatched allocator/deallocator
These are not mismatched, I'm using strdup() instead of strdup() for strings that must never be NULL.
Huh, what's the difference? Do strdup() behave differently than g_strdup()? (your sentence quoting the same function twice don't help much ;) )
Unless we have a policy to always use g_strdup()
We don't, but note that strdup() is not ISO C.
or there is a real g_/strdup vs. g_/free mismatch somewhere, I'm going to revert them.
There was at least 2 mismatches (and actually cppcheck pointed me at the first one); but maybe the other were fine.
Personally I prefer offensive programming: if something goes wrong, let it crash and burn, get a proper backtrace and fix it.
Agreed, but so? AFAIK, strdup() may return NULL upon allocation error; but g_strdup() won't ever return NULL unless the input string was itself NULL, since it uses g_malloc() which will abort upon allocation failure.
Regards, Colomban
On Thu, 07 Mar 2013 20:28:30 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
Log Message:
Scope: Fix mismatched allocator/deallocator
These are not mismatched, I'm using strdup() instead of strdup() for strings that must never be NULL.
Huh, what's the difference? Do strdup() behave differently than g_strdup()? (your sentence quoting the same function twice don't help much ;) )
Sorry, I meant "strdup() instead of g_strdup() for strings that must never be NULL". The difference is that strdup(NULL) crashes.
Unless we have a policy to always use g_strdup()...
We don't, but note that strdup() is not ISO C.
That's a valid argument, though both *NIX and Win~1 have it.
...or there is a real g_/strdup vs. g_/free mismatch somewhere
For example, an obvious mismatch is
foo ? utils_get_locale_from_utf8(start) : strdup(start)
utils_get_locale_from_utf8() allocates with glib, strdup() with libc.
Gee. OK, g_strdup() stays. It may be a bit harder to debug these, but it's not a bug issue, and certainly not worth mismatches.
Le 07/03/2013 21:41, Dimitar Zhekov a écrit :
On Thu, 07 Mar 2013 20:28:30 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
Log Message:
Scope: Fix mismatched allocator/deallocator
These are not mismatched, I'm using strdup() instead of strdup() for strings that must never be NULL.
Huh, what's the difference? Do strdup() behave differently than g_strdup()? (your sentence quoting the same function twice don't help much ;) )
Sorry, I meant "strdup() instead of g_strdup() for strings that must never be NULL".
I guessed that :)
The difference is that strdup(NULL) crashes.
...but strdup() may return NULL because of allocation failure. Maybe just wrap that if you mind, somewhat like Matthew said:
gchar *scope_strdup(const gchar *str) { g_return_val_if_fail(str != NULL, NULL); return g_strdup(str); }
...and you get free debugging hooks on g_return_val_if_fail() :)
Unless we have a policy to always use g_strdup()...
We don't, but note that strdup() is not ISO C.
That's a valid argument, though both *NIX and Win~1 have it.
Yeah practically it's probably not a real concern.
...or there is a real g_/strdup vs. g_/free mismatch somewhere
For example, an obvious mismatch is
foo ? utils_get_locale_from_utf8(start) : strdup(start)
utils_get_locale_from_utf8() allocates with glib, strdup() with libc.
Gee. OK, g_strdup() stays. It may be a bit harder to debug these,
Would it really change much that strdup() succeed but returned NULL, which would probably fail just after if the code didn't handle NULL? It's a wild hope anyway IMHO.
but it's not a bug issue, and certainly not worth mismatches.
...although in practice it's unlikely to be an issue in !win32.
Regards, Colomban
On 13-03-07 11:06 AM, Dimitar Zhekov wrote:
On Mon, 04 Mar 2013 22:30:40 -0000 Colomban Wendling git-noreply@geany.org wrote:
Log Message:
Scope: Fix mismatched allocator/deallocator
These are not mismatched, I'm using strdup() instead of strdup() for strings that must never be NULL. Unless we have a policy to always use g_strdup(), or there is a real g_/strdup vs. g_/free mismatch somewhere, I'm going to revert them.
I don't see any mismatch either but strdup() is not in C standard (at least C89/99) but rather POSIX or some other one AFAIK.
If you really don't like the g_strdup() behaviour, you could always write something like:
char* scope_strdup(const char *s) { char *n; size_t len; if (s == NULL) g_error("NULL string passed to scope_strdup()"); len = strlen(s) + 1; n = malloc(len); memcpy(n, s, len); return n; }
Then at least there's some hint of what went wrong rather than just bringing down the entire program with a mysterious segfault (assuming the above code I didn't test doesn't do that itself :).
Personally I prefer offensive programming: if something goes wrong, let it crash and burn, get a proper backtrace and fix it.
Note to self: don't use Scope plugin until all files are saved :)
Cheers, Matthew Brush
Le 07/03/2013 20:37, Matthew Brush a écrit :
On 13-03-07 11:06 AM, Dimitar Zhekov wrote:
On Mon, 04 Mar 2013 22:30:40 -0000 Colomban Wendling git-noreply@geany.org wrote:
Log Message:
Scope: Fix mismatched allocator/deallocator
These are not mismatched, I'm using strdup() instead of strdup() for strings that must never be NULL. Unless we have a policy to always use g_strdup(), or there is a real g_/strdup vs. g_/free mismatch somewhere, I'm going to revert them.
I don't see any mismatch either but strdup() is not in C standard (at least C89/99) but rather POSIX or some other one AFAIK.
For example, an obvious mismatch is
foo ? utils_get_locale_from_utf8(start) : strdup(start)
utils_get_locale_from_utf8() allocates with glib, strdup() with libc.
Regards, Colomban
On Thu, 07 Mar 2013 11:37:50 -0800 Matthew Brush mbrush@codebrainz.ca wrote:
Personally I prefer offensive programming: if something goes wrong, let it crash and burn, get a proper backtrace and fix it.
Note to self: don't use Scope plugin until all files are saved :)
You should save and compile the source files at least. Otherwise, debugging may be confusing. ;)
On Fri, 8 Mar 2013 06:44:15 +1100 Lex Trotman elextr@gmail.com wrote:
In which case this plugin must be removed from G-P until you have a reasonable assurance it won't bring Geany down. You may prefer crash and burn, but you should not impose that on innocent G-P users.
All sorts of invalid glib/gtk calls can bring Geany down without any messages, the g_return_*() checks in the libraries are by no means complete. So, it depends on the exact value of "reasonable".
The drawbacks of defensive programming are that bugs tend to stay unfixed, sunce they just emit a message in ~/.xsession-errors, and nobody notices/cares (I have several of those since forever); the clumsier way to get a proper stack trace; and the unpredictable caller behaviour when a function is not executed because of a g_return_*() - it may crash in some random place, and then you will receive an error report - but a wrong one (if the log messages are missing), or less helpful (with them included).
On 8 March 2013 08:07, Dimitar Zhekov dimitar.zhekov@gmail.com wrote:
On Thu, 07 Mar 2013 11:37:50 -0800 Matthew Brush mbrush@codebrainz.ca wrote:
Personally I prefer offensive programming: if something goes wrong, let it crash and burn, get a proper backtrace and fix it.
Note to self: don't use Scope plugin until all files are saved :)
You should save and compile the source files at least. Otherwise, debugging may be confusing. ;)
On Fri, 8 Mar 2013 06:44:15 +1100 Lex Trotman elextr@gmail.com wrote:
In which case this plugin must be removed from G-P until you have a reasonable assurance it won't bring Geany down. You may prefer crash and burn, but you should not impose that on innocent G-P users.
All sorts of invalid glib/gtk calls can bring Geany down without any messages, the g_return_*() checks in the libraries are by no means complete. So, it depends on the exact value of "reasonable".
I'm only complaining about the *deliberate* crash and burn policy you seemed to be espousing.
We should all try not to crash and burn in most reasonable circumstances.
But running out of memory is a situation that is not "reasonable" since you can't guarantee to get a message out or anything, so crash in the allocator is the better solution. Struggling on with a NULL is probably going to be a waste of development time.
The drawbacks of defensive programming are that bugs tend to stay unfixed, sunce they just emit a message in ~/.xsession-errors, and nobody notices/cares (I have several of those since forever); the clumsier way to get a proper stack trace; and the unpredictable caller behaviour when a function is not executed because of a g_return_*() - it may crash in some random place, and then you will receive an error report - but a wrong one (if the log messages are missing), or less helpful (with them included).
Our failure to notice messages and fix them isn't the users problem.
Making debugging easy by losing user files isn't a good tradeoff IMHO :)
Cheers Lex
-- E-gards: Jimmy _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On Fri, 8 Mar 2013 08:54:32 +1100 Lex Trotman elextr@gmail.com wrote:
Personally I prefer offensive programming: if something goes wrong, let it crash and burn, get a proper backtrace and fix it.
I'm only complaining about the *deliberate* crash and burn policy you seemed to be espousing.
We should all try not to crash and burn in most reasonable circumstances.
It's not like I'll crash the plugin because of invalid user input, errors in communication with gdb, incomplete/buggy gdb messages, or something like that - these are actually checked very strictly. What I don't do is to "defend" against programming errors in the plugin itself - for example, return 0 if the scope_array_get_element_size() emulations fails (that'll cause and endless loop).
I used the Linux version for a month without a single crash or other bug, and think that it's reasonably safe (found a minor flaw, but we are in freeze now). There may be bugs, of course, and even crashes, but you should save your source files and compile them before debugging anyway. So you'd better Save All, especially under Windows. That's it.
On 8 March 2013 06:06, Dimitar Zhekov dimitar.zhekov@gmail.com wrote:
On Mon, 04 Mar 2013 22:30:40 -0000 Colomban Wendling git-noreply@geany.org wrote:
Log Message:
Scope: Fix mismatched allocator/deallocator
These are not mismatched, I'm using strdup() instead of strdup() for strings that must never be NULL. Unless we have a policy to always use g_strdup(), or there is a real g_/strdup vs. g_/free mismatch somewhere, I'm going to revert them.
Personally I prefer offensive programming: if something goes wrong, let it crash and burn, get a proper backtrace and fix it.
In which case this plugin must be removed from G-P until you have a reasonable assurance it won't bring Geany down. You may prefer crash and burn, but you should not impose that on innocent G-P users.
Cheers Lex
-- E-gards: Jimmy _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Le 07/03/2013 20:44, Lex Trotman a écrit :
On 8 March 2013 06:06, Dimitar Zhekov dimitar.zhekov@gmail.com wrote:
On Mon, 04 Mar 2013 22:30:40 -0000 Colomban Wendling git-noreply@geany.org wrote:
Log Message:
Scope: Fix mismatched allocator/deallocator
These are not mismatched, I'm using strdup() instead of strdup() for strings that must never be NULL. Unless we have a policy to always use g_strdup(), or there is a real g_/strdup vs. g_/free mismatch somewhere, I'm going to revert them.
Personally I prefer offensive programming: if something goes wrong, let it crash and burn, get a proper backtrace and fix it.
In which case this plugin must be removed from G-P until you have a reasonable assurance it won't bring Geany down. You may prefer crash and burn, but you should not impose that on innocent G-P users.
I don't think he was saying that his plugin will crash rather than accept invalid input, but he prefers it to crash rather than pretend it worked but use e.g. invalid memory.
And Geany itself uses GLib, which means that memory exhaustion, if reported by the allocator, will lead to a abort() anyway, so I don't see any problem not dealing with NULL in most cases (e.g. when doing normal stuff, not allocating 4GB of RAM, in which case you not only should use g_try_malloc() but also rethink your program :D)
Regards, Colomban