Re: [Geany-Devel] [geany/geany-plugins] 4cc119: Scope: Fix mismatched allocator/deallocator

2013-03-07 Thread Matthew Brush

On 13-03-07 11:06 AM, Dimitar Zhekov wrote:

On Mon, 04 Mar 2013 22:30:40 -
Colomban Wendling git-nore...@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

___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] [geany/geany-plugins] 4cc119: Scope: Fix mismatched allocator/deallocator

2013-03-07 Thread Lex Trotman
On 8 March 2013 06:06, Dimitar Zhekov dimitar.zhe...@gmail.com wrote:
 On Mon, 04 Mar 2013 22:30:40 -
 Colomban Wendling git-nore...@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
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] [geany/geany-plugins] 4cc119: Scope: Fix mismatched allocator/deallocator

2013-03-07 Thread Colomban Wendling
Le 07/03/2013 20:44, Lex Trotman a écrit :
 On 8 March 2013 06:06, Dimitar Zhekov dimitar.zhe...@gmail.com wrote:
 On Mon, 04 Mar 2013 22:30:40 -
 Colomban Wendling git-nore...@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
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] [geany/geany-plugins] 4cc119: Scope: Fix mismatched allocator/deallocator

2013-03-07 Thread Colomban Wendling
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 -
 Colomban Wendling git-nore...@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
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] [geany/geany-plugins] 4cc119: Scope: Fix mismatched allocator/deallocator

2013-03-07 Thread Lex Trotman
On 8 March 2013 08:07, Dimitar Zhekov dimitar.zhe...@gmail.com wrote:
 On Thu, 07 Mar 2013 11:37:50 -0800
 Matthew Brush mbr...@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 ele...@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
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel