Re: [elinks-dev] [PATCH] error.c: fix gcc warning (vasprintf)

2007-04-20 Thread Kalle Olavi Niemitalo 
Alexey Tourbin [EMAIL PROTECTED] writes:

 + rv = vasprintf((char **) buf, fmt, params);
 + if (rv  0)
 + buf = fmt;
   va_end(params);
   elinks_internal(assertion failed: %s, buf);
   if (buf) free(buf);

It can then call free(fmt) and probably get a SIGSEGV.
Would you be happy with the following?

diff --git a/src/util/error.c b/src/util/error.c
index 34e4c88..4e6afe5 100644
--- a/src/util/error.c
+++ b/src/util/error.c
@@ -147,17 +147,17 @@ elinks_assertm(int x, unsigned char *fmt, ...)
unsigned char *buf = NULL;
va_list params;
 
if (assert_failed) return;
if (!(assert_failed = !x)) return;
 
va_start(params, fmt);
-   vasprintf((char **) buf, fmt, params);
+   (void) vasprintf((char **) buf, fmt, params);
va_end(params);
-   elinks_internal(assertion failed: %s, buf);
+   elinks_internal(assertion failed: %s, buf ? buf : fmt);
if (buf) free(buf);
 }
 
 
 #ifdef CONFIG_DEBUG
 void
 force_dump(void)


pgpwrMcXELdv9.pgp
Description: PGP signature
___
elinks-dev mailing list
elinks-dev@linuxfromscratch.org
http://linuxfromscratch.org/mailman/listinfo/elinks-dev

Re: [elinks-dev] [PATCH] error.c: fix gcc warning (vasprintf)

2007-04-16 Thread Alexey Tourbin 
On Tue, Feb 27, 2007 at 10:43:13PM +0200, Kalle Olavi Niemitalo wrote:
 Alexey Tourbin [EMAIL PROTECTED] writes:
 
  va_start(params, fmt);
  -   vasprintf((char **) buf, fmt, params);
  +   rv = vasprintf((char **) buf, fmt, params);
  +   if (rv  0) {
  +   perror(vasprintf);
  +   return;
  +   }
  va_end(params);
  elinks_internal(assertion failed: %s, buf);
 
 This is wrong.  va_end and elinks_internal must be called even if
 vasprintf fails.
 
 Possibilities to be handled:
 (a) vasprintf returns an error and doesn't alter buf.  Because
 buf is initialized as NULL, this is the same as (b).
 (b) vasprintf returns an error and sets buf = NULL.  This NULL
 should not be passed to elinks_internal because it will go
 to snprintf from there and perhaps cause a crash.  The NULL
 could be replaced with an empty string.  Displaying the
 error from vasprintf may be useful but may also distract
 users into reporting that error instead of the file name and
 line number.
 (c) vasprintf returns an error but sets buf != NULL.  In this
 case the string probably contains something sensible so it
 should be used.
 (d) vasprintf returns success but sets buf = NULL.  That should
 not be possible.
 (e) vasprintf returns success and sets buf != NULL.  This is the
 usual case.

commit 021a1d43da2db001436d777eeb2f533f011aa471
Author: Alexey Tourbin [EMAIL PROTECTED]
Date:   Sun Oct 15 17:42:31 2006 +0400

error.c: fix gcc warning (vasprintf)
(cherry picked from commit 55dff3d9c23e2c8f3358fa265fc4276616713ed5)

Actually if vasprintf fails, use plain fmt string.

diff --git a/src/util/error.c b/src/util/error.c
index 34e4c88..ba6b0ee 100644
--- a/src/util/error.c
+++ b/src/util/error.c
@@ -145,13 +145,16 @@ void
 elinks_assertm(int x, unsigned char *fmt, ...)
 {
unsigned char *buf = NULL;
+   int rv;
va_list params;
 
if (assert_failed) return;
if (!(assert_failed = !x)) return;
 
va_start(params, fmt);
-   vasprintf((char **) buf, fmt, params);
+   rv = vasprintf((char **) buf, fmt, params);
+   if (rv  0)
+   buf = fmt;
va_end(params);
elinks_internal(assertion failed: %s, buf);
if (buf) free(buf);


pgpHbka1AH6So.pgp
Description: PGP signature
___
elinks-dev mailing list
elinks-dev@linuxfromscratch.org
http://linuxfromscratch.org/mailman/listinfo/elinks-dev

Re: [elinks-dev] [PATCH] error.c: fix gcc warning (vasprintf)

2007-02-27 Thread Kalle Olavi Niemitalo 
Alexey Tourbin [EMAIL PROTECTED] writes:

   va_start(params, fmt);
 - vasprintf((char **) buf, fmt, params);
 + rv = vasprintf((char **) buf, fmt, params);
 + if (rv  0) {
 + perror(vasprintf);
 + return;
 + }
   va_end(params);
   elinks_internal(assertion failed: %s, buf);

This is wrong.  va_end and elinks_internal must be called even if
vasprintf fails.

Possibilities to be handled:
(a) vasprintf returns an error and doesn't alter buf.  Because
buf is initialized as NULL, this is the same as (b).
(b) vasprintf returns an error and sets buf = NULL.  This NULL
should not be passed to elinks_internal because it will go
to snprintf from there and perhaps cause a crash.  The NULL
could be replaced with an empty string.  Displaying the
error from vasprintf may be useful but may also distract
users into reporting that error instead of the file name and
line number.
(c) vasprintf returns an error but sets buf != NULL.  In this
case the string probably contains something sensible so it
should be used.
(d) vasprintf returns success but sets buf = NULL.  That should
not be possible.
(e) vasprintf returns success and sets buf != NULL.  This is the
usual case.


pgpkC0sNPSKBx.pgp
Description: PGP signature
___
elinks-dev mailing list
elinks-dev@linuxfromscratch.org
http://linuxfromscratch.org/mailman/listinfo/elinks-dev