Skip to contentSkip to conversation

package/urg: fix build

PATCH New Samuel Martin 3 participants 16 messages Sep 12, 2016 – Oct 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 package/urg: fix build

Update select-h patch to also fix the C++ serial driver.

Fixes:
http://autobuild.buildroot.net/results/cd7/cd76699188d2ee99b5f2d32b6a797de0d832a192/

---
 package/urg/0001-select-h.patch | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)
package/urg/0001-select-h.patch +22 -5
index 09d1530..46bb102 100644
@@ -1,11 +1,28 @@
---- urg-0.8.18/src/c/connection/serial_ctrl_lin.c	2010-11-22 01:48:01.000000000 +0000
-+++ urg-0.8.18.mod/src/c/connection/serial_ctrl_lin.c	2013-05-21 20:56:35.822025969 +0100
-@@ -17,7 +17,7 @@
- #include <fcntl.h>
+Serial driver: add missing header to C and C++ serial driver
+
+Fixes:
+  http://autobuild.buildroot.net/results/cd7/cd76699188d2ee99b5f2d32b6a797de0d832a192/
+
+Signed-off-by: Samuel Martin <[email protected]>
+
+--- a/src/c/connection/serial_ctrl_lin.c	2016-09-11 15:49:42.599807053 +0200
++++ b/src/c/connection/serial_ctrl_lin.c	2016-09-11 15:50:07.850700420 +0200
+@@ -18,6 +18,7 @@
  #include <errno.h>
  #include <string.h>
--
+ 
 +#include <sys/select.h>
  //#include <ctype.h>
  
  enum {
+--- a/src/cpp/connection/SerialDevice_lin.cpp	2016-09-11 15:38:23.335547059 +0200
++++ b/src/cpp/connection/SerialDevice_lin.cpp	2016-09-11 15:38:38.472741418 +0200
+@@ -13,7 +13,7 @@
+ #include <cerrno>
+ #include <cstring>
+ #include <cstdio>
+-
++#include <sys/select.h>
+ 
+ class RawSerialDevice
+ {
Signature · 1 line
2.9.3
#1 lore raw

PATCH 2/3 package/urg: fix build with gcc-6

Add patch fixing/adding explicit cast on negative value in char[], which is required with gcc-6 (see "Narrowing conversions" paragraph in [1])

Fixes:
http://autobuild.buildroot.net/results/d0c/d0cd11163753db69a14c02e941dbda40e5f98bba/

[1] https://gcc.gnu.org/gcc-6/porting_to.html

---
 .../0002-urg-gcc6-fix-narrowing-conversion.patch   | 99 ++++++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 package/urg/0002-urg-gcc6-fix-narrowing-conversion.patch
package/urg/0002-urg-gcc6-fix-narrowing-conversion.patch new file +99 -0
new file mode 100644
index 0000000..e3db898
@@ -0,0 +1,99 @@
+urg: fix 'narrowing conversion' with gcc6
+
+Fixes:
+  http://autobuild.buildroot.net/results/d0c/d0cd11163753db69a14c02e941dbda40e5f98bba/
+
+Signed-off-by: Samuel Martin <[email protected]>
+
+--- a/src/cpp/urg/ScipHandler.cpp	2016-09-11 16:11:16.083995214 +0200
++++ b/src/cpp/urg/ScipHandler.cpp	2016-09-11 16:11:24.380832543 +0200
+@@ -120,7 +120,7 @@ struct ScipHandler::pImpl
+ 
+       // QT ?????s
+       int return_code = -1;
+-      char qt_expected_response[] = { 0, -1 };
++      char qt_expected_response[] = { 0, (char)-1 };
+       // return_code ???g???????????AsetLaserOutput() ???p?????? QT ?????M????
+       if (response(return_code, "QT\n", qt_expected_response)) {
+         laser_state_ = LaserOff;
+@@ -139,7 +139,7 @@ struct ScipHandler::pImpl
+ 
+       } else if (return_code == Scip11Response) {
+         // SCIP1.1 ?v???g?R?????????????ASCIP2.0 ?????M????
+-        char scip20_expected_response[] = { 0, -1 };
++        char scip20_expected_response[] = { 0, (char)-1 };
+         if (! response(return_code, "SCIP2.0\n", scip20_expected_response)) {
+           error_message_ =
+             "SCIP1.1 protocol is not supported. Please update URG firmware, or reconnect after a few seconds because sensor is booting.";
+@@ -150,7 +150,7 @@ struct ScipHandler::pImpl
+ 
+       } else if (return_code == 0xE) {
+         // TM ???[?h?????????ATM2 ?????s????
+-        char tm2_expected_response[] = { 0, -1 };
++        char tm2_expected_response[] = { 0, (char)-1 };
+         if (response(return_code, "TM2\n", tm2_expected_response)) {
+           laser_state_ = LaserOff;
+           return changeBothBaudrate(baudrate);
+@@ -202,7 +202,7 @@ struct ScipHandler::pImpl
+     snprintf(send_buffer, 10, "SS%06ld\n", baudrate);
+     int return_code = -1;
+     // !!! ???????????????{?[???[?g?A?????????????l?? ss_expected... ??????????
+-    char ss_expected_response[] = { 0, 0x3, 0x4, 0xf, -1 };
++    char ss_expected_response[] = { 0, 0x3, 0x4, 0xf, (char)-1 };
+     if (! response(return_code, send_buffer, ss_expected_response)) {
+       error_message_ = "Baudrate change fail.";
+       return false;
+@@ -216,7 +216,7 @@ struct ScipHandler::pImpl
+   {
+     // PP ?????M???f?[?^?????M
+     int return_code = -1;
+-    char pp_expected_response[] = { 0, -1 };
++    char pp_expected_response[] = { 0, (char)-1 };
+     vector<string> lines;
+     if (! response(return_code, "PP\n", pp_expected_response, &lines)) {
+       error_message_ = "PP fail.";
+@@ -356,7 +356,7 @@ struct ScipHandler::pImpl
+ 
+     if (on) {
+       int return_code = -1;
+-      char expected_response[] = { 0, -1 };
++      char expected_response[] = { 0, (char)-1 };
+       if (! response(return_code, "BM\n", expected_response)) {
+         error_message_ = "BM fail.";
+         return false;
+@@ -369,7 +369,7 @@ struct ScipHandler::pImpl
+       if (! mx_capturing_) {
+         // ?????????????? QT ?????A??????????????
+         int return_code = -1;
+-        char qt_expected_response[] = { 0, -1 };
++        char qt_expected_response[] = { 0, (char)-1 };
+         if (! response(return_code, "QT\n", qt_expected_response)) {
+           return false;
+         }
+@@ -777,7 +777,7 @@ bool ScipHandler::loadParameter(RangeSen
+ bool ScipHandler::versionLines(vector<string>& lines)
+ {
+   int return_code = -1;
+-  char expected_response[] = { 0, -1 };
++  char expected_response[] = { 0, (char)-1 };
+   if (! pimpl->response(return_code, "VV\n", expected_response, &lines)) {
+     return false;
+   }
+@@ -792,7 +792,7 @@ bool ScipHandler::setRawTimestampMode(bo
+ 
+   // TM0 or TM2 ?????M
+   int return_code = -1;
+-  char expected_response[] = { 0, -1 };
++  char expected_response[] = { 0, (char)-1 };
+   if (! pimpl->response(return_code, send_command, expected_response)) {
+     pimpl->error_message_ = (on) ? "TM0 fail." : "TM2 fail.";
+     return false;
+@@ -809,7 +809,7 @@ bool ScipHandler::rawTimestamp(int* time
+ {
+   // TM1 ???l??????
+   int return_code = -1;
+-  char expected_response[] = { 0, -1 };
++  char expected_response[] = { 0, (char)-1 };
+   vector<string> lines;
+   if (! pimpl->response(return_code, "TM1\n", expected_response, &lines)) {
+     pimpl->error_message_ = "TM1 fail.";
Signature · 1 line
2.9.3
#2 lore raw

PATCH 2/3 package/urg: fix build with gcc-6

Hello,

Quoted 9 lines · Samuel Martin

Add patch fixing/adding explicit cast on negative value in char[], which is required with gcc-6 (see "Narrowing conversions" paragraph in [1])

Fixes:
http://autobuild.buildroot.net/results/d0c/d0cd11163753db69a14c02e941dbda40e5f98bba/

[1] https://gcc.gnu.org/gcc-6/porting_to.html

I don't see this patch in patchwork. Did you remove it from patchwork somehow ? Or was patchwork confused by something in your patch ?

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
#3 lore raw

PATCH 2/3 package/urg: fix build with gcc-6

Hi all,

Quoted 9 lines · Thomas Petazzoni

Hello,

Quoted 6 lines · Samuel Martin

Add patch fixing/adding explicit cast on negative value in char[], which is required with gcc-6 (see "Narrowing conversions" paragraph in [1])

Fixes:
http://autobuild.buildroot.net/results/d0c/d0cd11163753db69a14c02e941dbda40e5f98bba/

Quoted 6 lines
Quoted 3 lines

[1] https://gcc.gnu.org/gcc-6/porting_to.html

I don't see this patch in patchwork. Did you remove it from patchwork somehow ? Or was patchwork confused by something in your patch ?

Nope, I did not do anything in patchwork, it should appear...

Regards,

Signature · 1 line
Samuel
#4 lore raw

PATCH 2/3 package/urg: fix build with gcc-6

Quoted 13 lines · Samuel Martin

Hi all,

Quoted 9 lines · Thomas Petazzoni

Hello,

Quoted 6 lines · Samuel Martin

Add patch fixing/adding explicit cast on negative value in char[], which is required with gcc-6 (see "Narrowing conversions" paragraph in [1])

Fixes:
http://autobuild.buildroot.net/results/d0c/d0cd11163753db69a14c02e941dbda40e5f98bba/

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

[1] https://gcc.gnu.org/gcc-6/porting_to.html

I don't see this patch in patchwork. Did you remove it from patchwork somehow ? Or was patchwork confused by something in your patch ?

Nope, I did not do anything in patchwork, it should appear...

I expect that patchwork gets confused by the weird non-Unicode characters in
the patch and barfs. I remember seeing some patches on the patchwork mailing list that should improve that, but they're probably not applied to the ozlabs instance.

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#5 lore raw

PATCH 2/3 package/urg: fix build with gcc-6

In reply to Samuel Martin

Hello,

Quoted 6 lines · Samuel Martin

Add patch fixing/adding explicit cast on negative value in char[], which is required with gcc-6 (see "Narrowing conversions" paragraph in [1])

Fixes:
http://autobuild.buildroot.net/results/d0c/d0cd11163753db69a14c02e941dbda40e5f98bba/

Quoted 7 lines

[1] https://gcc.gnu.org/gcc-6/porting_to.html

---
 .../0002-urg-gcc6-fix-narrowing-conversion.patch   | 99 ++++++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 package/urg/0002-urg-gcc6-fix-narrowing-conversion.patch

Applied to master, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#6 lore raw

PATCH 3/3 package/nginx-naxsi: add missing pcre dependency

In reply to Samuel Martin

Fixes:
http://autobuild.buildroot.net/results/9c7/9c7bad6831b09251af81e2bbfc595a241df87c70/

---
 package/nginx-naxsi/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/nginx-naxsi/Config.in +1 -0
index 2f7c2da..6c175eb 100644
@@ -1,5 +1,6 @@
 config BR2_PACKAGE_NGINX_NAXSI
 	bool "nginx-naxsi"
+	select BR2_PACKAGE_PCRE
 	help
 	  NAXSI means Nginx Anti XSS & SQL Injection.
 
Signature · 1 line
2.9.3
#7 lore raw

PATCH 3/3 package/nginx-naxsi: add missing pcre dependency

Hello,

Quoted 7 lines · Samuel Martin

Fixes:
http://autobuild.buildroot.net/results/9c7/9c7bad6831b09251af81e2bbfc595a241df87c70/

---
 package/nginx-naxsi/Config.in | 1 +
 1 file changed, 1 insertion(+)

Applied to master, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#8 lore raw

PATCH 3/3 package/nginx-naxsi: add missing pcre dependency

In reply to Samuel Martin
Quoted 13 lines · Samuel Martin

Fixes:
http://autobuild.buildroot.net/results/9c7/9c7bad6831b09251af81e2bbfc595a241df87c70/

---
 package/nginx-naxsi/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/nginx-naxsi/Config.in +0 -0
index 2f7c2da..6c175eb 100644

package/nginx-naxsi/Config.in line 5

Quoted 5 lines
package/nginx-naxsi/Config.in +1 -0
@@ -1,5 +1,6 @@
 config BR2_PACKAGE_NGINX_NAXSI
 	bool "nginx-naxsi"
+	select BR2_PACKAGE_PCRE
 	help
 	  NAXSI means Nginx Anti XSS & SQL Injection.

I may be missing something, but shouldn't there be a _DEPENDENCIES += in .mk?

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#9 lore raw

PATCH 3/3 package/nginx-naxsi: add missing pcre dependency

Quoted 11 lines · Arnout Vandecappelle
Quoted 8 lines · Samuel Martin

Fixes:
http://autobuild.buildroot.net/results/9c7/9c7bad6831b09251af81e2bbfc595a241df87c70/

---
 package/nginx-naxsi/Config.in | 1 +
 1 file changed, 1 insertion(+)
Quoted 12 lines

package/nginx-naxsi/Config.in line 5

Quoted 10 lines
package/nginx-naxsi/Config.in +1 -0
index 2f7c2da..6c175eb 100644
@@ -1,5 +1,6 @@
 config BR2_PACKAGE_NGINX_NAXSI
      bool "nginx-naxsi"
+     select BR2_PACKAGE_PCRE
      help
        NAXSI means Nginx Anti XSS & SQL Injection.

I may be missing something, but shouldn't there be a _DEPENDENCIES += in .mk?

Nope, this is not needed.
naxsi is a nginx' module, built by nginx.mk, which already has an automatic dependency on pcre when selected.

Regards,

Signature · 1 line
Samuel
#10 lore raw

PATCH 3/3 package/nginx-naxsi: add missing pcre dependency

Quoted 25 lines · Samuel Martin
Quoted 11 lines · Arnout Vandecappelle
Quoted 8 lines · Samuel Martin

Fixes:
http://autobuild.buildroot.net/results/9c7/9c7bad6831b09251af81e2bbfc595a241df87c70/

---
 package/nginx-naxsi/Config.in | 1 +
 1 file changed, 1 insertion(+)
Quoted 12 lines

package/nginx-naxsi/Config.in line 5

Quoted 10 lines
package/nginx-naxsi/Config.in +1 -0
index 2f7c2da..6c175eb 100644
@@ -1,5 +1,6 @@
 config BR2_PACKAGE_NGINX_NAXSI
      bool "nginx-naxsi"
+     select BR2_PACKAGE_PCRE
      help
        NAXSI means Nginx Anti XSS & SQL Injection.

I may be missing something, but shouldn't there be a _DEPENDENCIES += in .mk?

Quoted 3 lines

Nope, this is not needed.
naxsi is a nginx' module, built by nginx.mk, which already has an automatic dependency on pcre when selected.

I don't think we should rely on transitive dependencies. naxsi.h #include's
pcre.h, so there should be an explicit dependency in the .mk file.

Whenever you don't have that, the 'select' in Config.in should carry a comment
explaining why not (usually 'runtime dependency').

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
#11 lore raw

PATCH 3/3 package/nginx-naxsi: add missing pcre dependency

Quoted 28 lines · Arnout Vandecappelle
Quoted 25 lines · Samuel Martin
Quoted 23 lines · Arnout Vandecappelle
Quoted 13 lines · Samuel Martin

Fixes:
http://autobuild.buildroot.net/results/9c7/9c7bad6831b09251af81e2bbfc595a241df87c70/

---
 package/nginx-naxsi/Config.in | 1 +
 1 file changed, 1 insertion(+)
package/nginx-naxsi/Config.in +0 -0
index 2f7c2da..6c175eb 100644

package/nginx-naxsi/Config.in line 5

Quoted 5 lines
package/nginx-naxsi/Config.in +1 -0
@@ -1,5 +1,6 @@
 config BR2_PACKAGE_NGINX_NAXSI
      bool "nginx-naxsi"
+     select BR2_PACKAGE_PCRE
      help
        NAXSI means Nginx Anti XSS & SQL Injection.

I may be missing something, but shouldn't there be a _DEPENDENCIES += in .mk?

Quoted 6 lines
Quoted 3 lines

Nope, this is not needed.
naxsi is a nginx' module, built by nginx.mk, which already has an automatic dependency on pcre when selected.

I don't think we should rely on transitive dependencies. naxsi.h #include's
pcre.h, so there should be an explicit dependency in the .mk file.

Fair enough, I'll post the follow-up shortly.

Signature · 1 line
Samuel
#12 lore raw

PATCH 3/3 package/nginx-naxsi: add missing pcre dependency

In reply to Arnout Vandecappelle

Hello,

Quoted 9 lines · Arnout Vandecappelle
Quoted 3 lines

Nope, this is not needed.
naxsi is a nginx' module, built by nginx.mk, which already has an automatic dependency on pcre when selected.

I don't think we should rely on transitive dependencies. naxsi.h #include's
pcre.h, so there should be an explicit dependency in the .mk file.

Whenever you don't have that, the 'select' in Config.in should carry a comment
explaining why not (usually 'runtime dependency').

It's not a transitive dependency here. The interaction between nginx-naxsi and nginx is very special: nginx-naxsi is only in charge of downloading source code, and provide it to nginx during its build process. nginx is the one that *depends* on nginx-naxsi, so that nginx-naxsi has downloaded and extracted its source code before nginx gets configured.

So, nginx-naxsi is the one that really needs pcre (so it makes sense for it to select pcre). But it terms of build ordering, pcre is only really needed before nginx starts its configuration step.

So I believe the current situation is OK, with the exception that nginx-naxsi/Config.in should have a comment that explains that.

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
#13 lore raw

PATCH 1/3 package/urg: fix build

In reply to Samuel Martin

Hello,

Quoted 9 lines · Samuel Martin

Update select-h patch to also fix the C++ serial driver.

Fixes:
http://autobuild.buildroot.net/results/cd7/cd76699188d2ee99b5f2d32b6a797de0d832a192/

---
 package/urg/0001-select-h.patch | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

Applied to master, thanks.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#14 lore raw

PATCH 1/3 package/urg: fix build

Hello,

Quoted 8 lines · Thomas Petazzoni

Hello,

Quoted 5 lines · Samuel Martin

Update select-h patch to also fix the C++ serial driver.

Fixes:
http://autobuild.buildroot.net/results/cd7/cd76699188d2ee99b5f2d32b6a797de0d832a192/

Quoted 6 lines
Quoted 4 lines
---
 package/urg/0001-select-h.patch | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)  

Applied to master, thanks.

I've reverted, as the patch didn't apply, and caused all urg builds to fail:

http://autobuild.buildroot.net/results/a84/a84b63347716b62782e3b562049326f9285c785b/build-end.log

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
#15 lore raw

PATCH 1/3 package/urg: fix build

Hi Thomas,

Quoted 16 lines · Thomas Petazzoni

Hello,

Quoted 13 lines · Thomas Petazzoni

Hello,

Quoted 9 lines · Samuel Martin

Update select-h patch to also fix the C++ serial driver.

Fixes:
http://autobuild.buildroot.net/results/cd7/cd76699188d2ee99b5f2d32b6a797de0d832a192/

---
 package/urg/0001-select-h.patch | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)
Quoted 7 lines
Quoted 1 line

Applied to master, thanks.

I've reverted, as the patch didn't apply, and caused all urg builds to fail:

   http://autobuild.buildroot.net/results/a84/a84b63347716b62782e3b562049326f9285c785b/build-end.log

Indeed :(
You did right. I was not available yesterday to watch over the a.b.o. failures. I'll redo the the fix properly and post it shortly.

Regards,

Signature · 1 line
Samuel