Re: [Lynx-dev] Fixing some warnings.

2017-06-06 Thread Juan Manuel Guerrero 

Am 06.06.2017 02:15, schrieb Thomas Dickey:

On Mon, Jun 05, 2017 at 10:45:36PM +0200, Juan Manuel Guerrero wrote:

While I was checking the DJGPP code I have submitted, I have found certain
warnings/issues that I have tried to fix.  See patch below; it is only a
suggestion.


thanks (will review/etc)




OFYI, after having applied my DJGPP patch to the sources I have compiled the
code on a linux machine using gcc 6.1.0 with the options -Wall -pedantic.  I
was surprised about the warnings I got.  E.g.:

gcc -DHAVE_CONFIG_H  -DLOCALEDIR=\"/usr/local/share/locale\" -I. -I.. 
-Ichrtrans -I../../lynx2.8.9dev.14/src/chrtrans -I../../lynx2.8.9dev.14 
-I../../lynx2.8.9dev.14/src -I../../lynx2.8.9dev.14/./WWW/Library/Implementation
-D_GNU_SOURCE -D_DEFAULT_SOURCE -DLINUX -O2  -c ../../lynx2.8.9dev.14/src/HTML.c
../../lynx2.8.9dev.14/src/HTML.c: In function ‘HTML_start_element’:
../../lynx2.8.9dev.14/src/HTML.c:3714:28: warning: comparison between pointer 
and zero character constant [-Wpointer-compare]
   if (me->object_title == '\0') {
^~
../../lynx2.8.9dev.14/src/HTML.c:3714:11: note: did you mean to dereference the 
pointer?
   if (me->object_title == '\0') {
   ^

The code itself confuses me a little bit.  Either the intention is to check
against NULL pointer, then the original code is wrong.  IMHO it should look
like this:
  if (me->object_title == NULL) {


Or the intention is to check that the strings are empty before deallocating
them, then the code should look like this:
  if (me->object_title[0] == '\0') {


The reason why I have submitted the patch as it is, is because after having
inspected function HTML_start_element, I have found the following line:
  if (*me->object_usemap == '\0') {

that produces no compiler warnings, thus I have assumed that this was the
intention of the other lines.  I have adjusted them accordingly.  But to
be serious, I do not understand the goal of those lines at all.  If the goal
is to deallocate the strings, I do not understand why it is necessary to check
that they are not NULL pointers and why it is necessary to check that they are
empty.  Why is not called the free function directly with the pointer.
But again the patch is only the result of my observations when compiling
the code.  If the patch does make any sense can only be judge by the authors
of lynx.

Kind regards,
Juan M. Guerrero

___
Lynx-dev mailing list
Lynx-dev@nongnu.org
https://lists.nongnu.org/mailman/listinfo/lynx-dev

Re: [Lynx-dev] Fixing some warnings.

2017-06-05 Thread Thomas Dickey 
On Mon, Jun 05, 2017 at 10:45:36PM +0200, Juan Manuel Guerrero wrote:
> While I was checking the DJGPP code I have submitted, I have found certain
> warnings/issues that I have tried to fix.  See patch below; it is only a
> suggestion.

thanks (will review/etc)

-- 
Thomas E. Dickey 
http://invisible-island.net
ftp://invisible-island.net


signature.asc
Description: Digital signature
___
Lynx-dev mailing list
Lynx-dev@nongnu.org
https://lists.nongnu.org/mailman/listinfo/lynx-dev

Re: [Lynx-dev] Fixing some warnings.

2017-06-05 Thread Thorsten Glaser 
Juan Manuel Guerrero dixit:

> - if (me->object_title == '\0') {
> + if (*me->object_title == '\0') {
>   FREE(me->object_title);

I think a NULL pointer comparison was intended here, and that
this patch may introduce use-after-free bugs.

However, why FREE() doesn’t accept null pointers in the first
place is questionable. (And if free() is macro-wrapped already,
it should probably assign NULL to the variable afterwards, just
to be safe.)

bye,
//mirabilos
-- 
In traditional syntax ' is ignored, but in c99 everything between two ' is
handled as character constant.  Therefore you cannot use ' in a preproces-
sing file in c99 mode.  -- Ragge
No faith left in ISO C99, undefined behaviour, etc.

___
Lynx-dev mailing list
Lynx-dev@nongnu.org
https://lists.nongnu.org/mailman/listinfo/lynx-dev

[Lynx-dev] Fixing some warnings.

2017-06-05 Thread Juan Manuel Guerrero 

While I was checking the DJGPP code I have submitted, I have found certain
warnings/issues that I have tried to fix.  See patch below; it is only a
suggestion.


Regards,
Juan M. Guerrero




2017-06-05  Juan Manuel Guerrero  

* src/GridText.c (ALLOC_IN_POOL): Fix warning: 'ptr' may be used 
uninitialized
in this function [-Wmaybe-uninitialized].

src/HTML.c (HTML_start_element): Fix warning: comparison between pointer
and zero character constant [-Wpointer-compare].

src/LYMainLoop.c (check_JUMP_param): Fix warning: comparison between 
pointer
and zero character constant [-Wpointer-compare].








diff -aprNU5 lynx2.8.9dev.14.orig/src/GridText.c lynx2.8.9dev.14/src/GridText.c
--- lynx2.8.9dev.14.orig/src/GridText.c 2017-02-11 00:50:00 +
+++ lynx2.8.9dev.14/src/GridText.c  2017-06-05 20:37:10 +
@@ -238,11 +238,11 @@ There are 3 functions - POOL_NEW, POOL_F
  * Updates 'poolptr' if necessary.
  */
 static void *ALLOC_IN_POOL(HTPool ** ppoolptr, unsigned request)
 {
 HTPool *pool = *ppoolptr;
-pool_data *ptr;
+pool_data *ptr = NULL;
 unsigned n;
 unsigned j;

 if (!pool) {
outofmem(__FILE__, "ALLOC_IN_POOL");
diff -aprNU5 lynx2.8.9dev.14.orig/src/HTML.c lynx2.8.9dev.14/src/HTML.c
--- lynx2.8.9dev.14.orig/src/HTML.c 2017-04-30 18:45:06 +
+++ lynx2.8.9dev.14/src/HTML.c  2017-06-05 20:44:26 +
@@ -3709,11 +3709,11 @@ static int HTML_start_element(HTStructur
non_empty(value[HTML_OBJECT_TITLE])) {
StrAllocCopy(me->object_title, value[HTML_OBJECT_TITLE]);
TRANSLATE_AND_UNESCAPE_ENTITIES(>object_title, TRUE, 
FALSE);
LYTrimHead(me->object_title);
LYTrimTail(me->object_title);
-   if (me->object_title == '\0') {
+   if (*me->object_title == '\0') {
FREE(me->object_title);
}
}
if (present[HTML_OBJECT_DATA] &&
non_empty(value[HTML_OBJECT_DATA])) {
@@ -3727,22 +3727,22 @@ static int HTML_start_element(HTStructur
non_empty(value[HTML_OBJECT_TYPE])) {
StrAllocCopy(me->object_type, value[HTML_OBJECT_TYPE]);
TRANSLATE_AND_UNESCAPE_ENTITIES(>object_type, TRUE, 
FALSE);
LYTrimHead(me->object_type);
LYTrimTail(me->object_type);
-   if (me->object_type == '\0') {
+   if (*me->object_type == '\0') {
FREE(me->object_type);
}
}
if (present[HTML_OBJECT_CLASSID] &&
non_empty(value[HTML_OBJECT_CLASSID])) {
StrAllocCopy(me->object_classid,
 value[HTML_OBJECT_CLASSID]);
TRANSLATE_AND_UNESCAPE_ENTITIES(>object_classid, TRUE, 
FALSE);
LYTrimHead(me->object_classid);
LYTrimTail(me->object_classid);
-   if (me->object_classid == '\0') {
+   if (*me->object_classid == '\0') {
FREE(me->object_classid);
}
}
if (present[HTML_OBJECT_CODEBASE] &&
non_empty(value[HTML_OBJECT_CODEBASE])) {
@@ -3760,21 +3760,21 @@ static int HTML_start_element(HTStructur
TRANSLATE_AND_UNESCAPE_ENTITIES(>object_codetype,
TRUE,
FALSE);
LYTrimHead(me->object_codetype);
LYTrimTail(me->object_codetype);
-   if (me->object_codetype == '\0') {
+   if (*me->object_codetype == '\0') {
FREE(me->object_codetype);
}
}
if (present[HTML_OBJECT_NAME] &&
non_empty(value[HTML_OBJECT_NAME])) {
StrAllocCopy(me->object_name, value[HTML_OBJECT_NAME]);
TRANSLATE_AND_UNESCAPE_ENTITIES(>object_name, TRUE, 
FALSE);
LYTrimHead(me->object_name);
LYTrimTail(me->object_name);
-   if (me->object_name == '\0') {
+   if (*me->object_name == '\0') {
FREE(me->object_name);
}
}
}
/*
@@ -5065,11 +5065,11 @@ static int HTML_start_element(HTStructur

if (present && present[HTML_TEXTAREA_ID]
&& non_empty(value[HTML_TEXTAREA_ID])) {
StrAllocCopy(id_string, value[HTML_TEXTAREA_ID]);
TRANSLATE_AND_UNESCAPE_TO_STD(_string);
-   if ((id_string != '\0') &&
+   if ((*id_string != '\0') &&
(ID_A = HTAnchor_findChildAndLink(me->node_anchor,   /*