Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails to build due to problems with boolean types and constants #98

Closed
sebastic opened this issue Jun 2, 2022 · 5 comments
Closed

Fails to build due to problems with boolean types and constants #98

sebastic opened this issue Jun 2, 2022 · 5 comments

Comments

@sebastic
Copy link
Contributor

sebastic commented Jun 2, 2022

As reported in Debian Bug #1012231:

tinyows recently started to FTBFS after some build-dependency got updated:

   dh_auto_build
        make -j3
make[1]: Entering directory '/build/tinyows-1.2.0'
gcc -g -O2 -ffile-prefix-map=/build/tinyows-1.2.0=. -fstack-protector-strong -Wformat -Werror=format-security -Wl,-z,relro -Wl,-z,now -Wdate-time -D_FORTIFY_SOURCE=2 -std=c99 -pedantic -Wall -I/usr/include/postgre
sql -I/usr/include/libxml2   src/fe/fe_comparison_ops.c src/fe/fe_error.c src/fe/fe_filter.c src/fe/fe_filter_capabilities.c src/fe/fe_function.c src/fe/fe_logical_ops.c src/fe/fe_spatial_ops.c src/mapfile/mapfile
.c src/ows/ows_bbox.c src/ows/ows.c src/ows/ows_config.c src/ows/ows_error.c src/ows/ows_geobbox.c src/ows/ows_get_capabilities.c src/ows/ows_layer.c src/ows/ows_metadata.c src/ows/ows_psql.c src/ows/ows_request.c
 src/ows/ows_srs.c src/ows/ows_storage.c src/ows/ows_version.c src/struct/alist.c src/struct/array.c src/struct/buffer.c src/struct/cgi_request.c src/struct/list.c src/struct/mlist.c src/struct/regexp.c src/wfs/wf
s_describe.c src/wfs/wfs_error.c src/wfs/wfs_get_capabilities.c src/wfs/wfs_get_feature.c src/wfs/wfs_request.c src/wfs/wfs_transaction.c src/ows/ows_libxml.c -o tinyows -lfl -L/usr/lib/x86_64-linux-gnu -lpq -lxml
2 -lfcgi
In file included from /usr/include/unicode/umachine.h:52,
                 from /usr/include/unicode/utypes.h:38,
                 from /usr/include/unicode/ucnv_err.h:88,
                 from /usr/include/unicode/ucnv.h:51,
                 from /usr/include/libxml2/libxml/encoding.h:31,
                 from /usr/include/libxml2/libxml/parser.h:812,
                 from /usr/include/libxml2/libxml/globals.h:18,
                 from /usr/include/libxml2/libxml/threads.h:35,
                 from /usr/include/libxml2/libxml/xmlmemory.h:218,
                 from /usr/include/libxml2/libxml/tree.h:1307,
                 from /usr/include/libxml2/libxml/xmlreader.h:14,
                 from src/fe/../ows/ows.h:30,
                 from src/fe/fe_comparison_ops.c:29:
src/fe/../ows/../ows_struct.h:33:3: error: expected identifier before numeric constant
   33 |   false,
      |   ^~~~~
src/fe/../ows/../ows_struct.h:37:19: error: two or more data types in declaration specifiers
   37 | typedef enum Bool bool;
      |                   ^~~~
In file included from src/fe/../ows/ows.h:38,
                 from src/fe/fe_comparison_ops.c:29:
src/fe/../ows/../ows_struct.h:37:14: warning: empty declaration with storage class specifier does not redeclare tag
   37 | typedef enum Bool bool;
      |              ^~~~
[...]

Full buildlog

@sebastic
Copy link
Contributor Author

sebastic commented Jun 2, 2022

There seems to be a conflict between stdbool.h as included by ICU in umachine.h and the definitions in ows_struct.h.

This patch works around the issue:

--- tinyows-1.2.0.orig/src/ows_struct.h
+++ tinyows-1.2.0/src/ows_struct.h
@@ -29,12 +29,12 @@
 
 /* ========= Structures ========= */
 
-enum Bool {
-  false,
-  true
+enum Bool_ {
+  FALSE = 0,
+  TRUE = 1
 };
 
-typedef enum Bool bool;
+typedef enum Bool_ bool_;
 
 #define BUFFER_SIZE_INIT   256
 
@@ -107,7 +107,7 @@ typedef struct Ows_layer_storage {
   buffer * pkey;
   buffer * pkey_sequence;
   buffer * pkey_default;
-  bool is_geographic;    /* true for a geographic CRS (or a compound CRS
+  bool_ is_geographic;   /* true for a geographic CRS (or a compound CRS
                             whose base is geographic), false for a projected
                             CRS (or a compound CRS whose base is projected) */
   array * attributes;
@@ -117,11 +117,11 @@ typedef struct Ows_srs {
   int srid;
   buffer * auth_name;
   int auth_srid;
-  bool is_geographic;              /* true for a geographic CRS (or a compound
+  bool_ is_geographic;             /* true for a geographic CRS (or a compound
                                       CRS whose base is geographic), false for
                                       a projected CRS (or a compound CRS whose
                                       base is projected) */
-  bool is_axis_order_gis_friendly;   /* true for a CRS whose axis order is
+  bool_ is_axis_order_gis_friendly;  /* true for a CRS whose axis order is
                                         typically easting, northing (e.g most
                                         projected CRS, such as EPSG:32631)
                                         false for example for EPSG:4326 (WGS 84),
@@ -129,10 +129,10 @@ typedef struct Ows_srs {
 
   /* The two below fields are not properties of the SRS, but of its context
    * of use. */
-  bool honours_authority_axis_order; /* true for a context where the axis order
-                                        as defined by the authority should be
-                                        respected */
-  bool is_long;                      /* true for a context where the srsname must
+  bool_ honours_authority_axis_order; /* true for a context where the axis order
+                                         as defined by the authority should be
+                                         respected */
+  bool_ is_long;                     /* true for a context where the srsname must
                                         be exported as a long URN */
 } ows_srs;
 
@@ -184,8 +184,8 @@ typedef struct Ows_layer {
   buffer * name_prefix;     /* Nominally concatenation of ns_prefix:name_no_uri, e.g. "tows:world" , or name_no_uri if no ns_prefix */
   buffer * name_no_uri;     /* the name as in the "name" attribute in the config, e.g "world" */
   buffer * title;
-  bool retrievable;
-  bool writable;
+  bool_ retrievable;
+  bool_ writable;
   list * srid;
   ows_geobbox * geobbox;
   buffer * abstract;
@@ -346,8 +346,8 @@ enum fe_error_code {
 };
 
 typedef struct Filter_encoding {
-  bool in_not;
-  bool is_numeric;
+  bool_ in_not;
+  bool_ is_numeric;
   buffer * sql;
   enum fe_error_code error_code;
 } filter_encoding;
@@ -370,10 +370,10 @@ typedef struct Ows_request {
 #define OWS_MAX_DOUBLE 1e15  /* %f vs %g */
 
 typedef struct Ows {
-  bool init;
-  bool exit;
+  bool_ init;
+  bool_ exit;
   PGconn * pg;
-  bool mapfile;
+  bool_ mapfile;
   buffer * config_file;
   buffer * schema_dir;
   buffer * online_resource;
@@ -396,12 +396,12 @@ typedef struct Ows {
   int max_features;
   ows_geobbox * max_geobbox;
 
-  bool display_bbox;
-  bool expose_pk;
-  bool estimated_extent;
+  bool_ display_bbox;
+  bool_ expose_pk;
+  bool_ estimated_extent;
 
-  bool check_schema;
-  bool check_valid_geom;
+  bool_ check_schema;
+  bool_ check_valid_geom;
 
   array * cgi;
   list * psql_requests;

@sebastic
Copy link
Contributor Author

sebastic commented Jun 2, 2022

Using stdbool.h instead of the custom type is also an option:

--- tinyows-1.2.0.orig/src/ows_struct.h
+++ tinyows-1.2.0/src/ows_struct.h
@@ -24,18 +24,12 @@
 #ifndef OWS_STRUCT_H
 #define OWS_STRUCT_H
 
+#include <stdbool.h>
 #include <stdio.h>    /* FILE prototype */
 
 
 /* ========= Structures ========= */
 
-enum Bool {
-  false,
-  true
-};
-
-typedef enum Bool bool;
-
 #define BUFFER_SIZE_INIT   256
 
 typedef struct Buffer {

But this results in warnings:

src/fe/fe_logical_ops.c: In function 'fe_unary_logical_op':
src/fe/fe_logical_ops.c:104:13: warning: increment of a boolean expression [-Wbool-operation]
  104 |   fe->in_not++;
      |             ^~
src/fe/fe_logical_ops.c:115:13: warning: decrement of a boolean expression [-Wbool-operation]
  115 |   fe->in_not--;
      |             ^~

@jmckenna
Copy link
Member

jmckenna commented Jun 2, 2022

@sebastic using stdbool.h gives no such warnings here on Windows, with Visual Studio (and tested against PostgreSQL 14.3/PostGIS 3.2.1). This seems like the proper way (?).

@sebastic
Copy link
Contributor Author

sebastic commented Jun 2, 2022

Using stdbool.h with GCC 11 and -Wall on Debian unstable you do get the warnings.

fe_unary_logical_op will need to be changed to fix that:

 92 /*
 93  * If the logical operator is unary, go through the nodes recursively
 94  * and fill the SQL request buffer
 95  */
 96 static buffer *fe_unary_logical_op(ows * o, buffer * typename, filter_encoding * fe, xmlNodePtr n)
 97 {
 98   assert(typename);
 99   assert(o);
100   assert(fe);
101   assert(n);
102 
103   buffer_add_str(fe->sql, "not(");
104   fe->in_not++;
105 
106   n = n->children;
107   while (n->type != XML_ELEMENT_NODE) n = n->next;
108 
109   /* Execute the matching function's type */
110   if (fe_is_logical_op((char *) n->name))    fe->sql = fe_logical_op(o, typename, fe, n);
111   else if (fe_is_spatial_op((char *) n->name))    fe->sql = fe_spatial_op(o, typename, fe, n);
112   else if (fe_is_comparison_op((char *) n->name)) fe->sql = fe_comparison_op(o, typename, fe, n);
113 
114   buffer_add_str(fe->sql, ")");
115   fe->in_not--;
116 
117   return fe->sql;
118 }

The following may suffice if you're not trying to cycle through the enum values:

--- a/src/ows_struct.h
+++ b/src/ows_struct.h
@@ -24,18 +24,12 @@
 #ifndef OWS_STRUCT_H
 #define OWS_STRUCT_H
 
+#include <stdbool.h>
 #include <stdio.h>    /* FILE prototype */
 
 
 /* ========= Structures ========= */
 
-enum Bool {
-  false,
-  true
-};
-
-typedef enum Bool bool;
-
 #define BUFFER_SIZE_INIT   256
 
 typedef struct Buffer {
--- a/src/fe/fe_logical_ops.c
+++ b/src/fe/fe_logical_ops.c
@@ -21,6 +21,7 @@
 */
 
 
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -101,7 +102,7 @@ static buffer *fe_unary_logical_op(ows *
   assert(n);
 
   buffer_add_str(fe->sql, "not(");
-  fe->in_not++;
+  fe->in_not = true;
 
   n = n->children;
   while (n->type != XML_ELEMENT_NODE) n = n->next;
@@ -112,7 +113,7 @@ static buffer *fe_unary_logical_op(ows *
   else if (fe_is_comparison_op((char *) n->name)) fe->sql = fe_comparison_op(o, typename, fe, n);
 
   buffer_add_str(fe->sql, ")");
-  fe->in_not--;
+  fe->in_not = false;
 
   return fe->sql;
 }

@jmckenna
Copy link
Member

jmckenna commented Jun 4, 2022

@sebastic I'm now able to reproduce the warnings, and your last patch works here. Can you turn this into a PR ? (I refer to ows_struct.h/fe_logical_ops.c ). thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants