[Starlingx-discuss] openSUSE: sm-db build failure
Hi Bin, Based in our work for openSUSE [0], and due to rpmlint configuration, we are having 2 warnings treated as errors while building sm-db package: [ 4s] sm_db_service_heartbeat.c: In function 'SmErrorT sm_db_service_heartbeat_insert(SmDbHandleT*, SmDbServiceHeartbeatT*)': [ 4s] sm_db_service_heartbeat.c:321:13: error: format '%li' expects argument of type 'long int', but argument 35 has type 'SmTimerIdT {aka int}' [-Werror=format=] [ 4s] record->heartbeat_timer_id, [ 4s] sm_db_service_heartbeat.c: In function 'SmErrorT sm_db_service_heartbeat_update(SmDbHandleT*, SmDbServiceHeartbeatT*)': [ 4s] sm_db_service_heartbeat.c:438:49: error: format '%li' expects argument of type 'long int', but argument 5 has type 'SmTimerIdT {aka int}' [-Werror=format=] [ 4s] record->heartbeat_timer_id ); [ 4s] cc1plus: all warnings being treated as errors We have confirmed the package gets built after reverting commit "Enhance timer system to avoid double deregister" [1]. Can you please guide us into the right direction to get this properly fixed? [0] https://review.opendev.org/#/c/679686/ [1] https://review.opendev.org/#/c/675936/
OpenSUSE is pretty strict when it comes to the quality of the code which gets in their repo and enforce some compiler warnings to to upgraded as error. There are several ways to correct those issues: 1) fix the problem in the source The best and prefer option is to fix the code. These type errors can induce nasty side effect and often are easy to correct. 2) change the compiler option In the make file, select the the required option to ignore the error. If there is no warning from the compiler, OpenSUSE will assume (rightly or wrongly) that you know what you do and you had no choose in keeping such ugliness. 3) use rpmlinrc rpmlint calculate an ugliness marking and if you pass the threshold it fails the package. You can change the ugliness marks allocated to certain type of errors. It's often easier to go around the issue, when possible (not for all warning type) but as bad as solution 2). Conclusion. ------------------ It's often not longer to fix the code once for all. That is the prefer route. Dominig On 25/09/2019 02:55, Arce Moreno, Abraham wrote:
Hi Bin,
Based in our work for openSUSE [0], and due to rpmlint configuration, we are having 2 warnings treated as errors while building sm-db package:
[ 4s] sm_db_service_heartbeat.c: In function 'SmErrorT sm_db_service_heartbeat_insert(SmDbHandleT*, SmDbServiceHeartbeatT*)': [ 4s] sm_db_service_heartbeat.c:321:13: error: format '%li' expects argument of type 'long int', but argument 35 has type 'SmTimerIdT {aka int}' [-Werror=format=] [ 4s] record->heartbeat_timer_id, [ 4s] sm_db_service_heartbeat.c: In function 'SmErrorT sm_db_service_heartbeat_update(SmDbHandleT*, SmDbServiceHeartbeatT*)': [ 4s] sm_db_service_heartbeat.c:438:49: error: format '%li' expects argument of type 'long int', but argument 5 has type 'SmTimerIdT {aka int}' [-Werror=format=] [ 4s] record->heartbeat_timer_id ); [ 4s] cc1plus: all warnings being treated as errors
We have confirmed the package gets built after reverting commit "Enhance timer system to avoid double deregister" [1]. Can you please guide us into the right direction to get this properly fixed?
[0] https://review.opendev.org/#/c/679686/ [1] https://review.opendev.org/#/c/675936/
_______________________________________________ Starlingx-discuss mailing list Starlingx-discuss@lists.starlingx.io http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
-- Dominig ar Foll Senior Software Architect Intel Open Source Technology Centre
Dominig, I am wondering there is something not right in your build environment. The SmTimerIdT is defined as: typedef int64_t SmTimerIdT; in sm_timer.h [1] patch set 1..3, which is part of the sm-common lib. There are multiple printf statements in sm-common lib that use the formater "%li" such as [2], line 150, 201. I don't expect you see the same errors from sm-common build, or otherwise the sm-db won't start building as it needs sm-common. Based on the error message in your first email, apparently the SmTimerIdT was "int" ('SmTimerIdT {aka int}'). That was the code before my commit "Enhance timer system to avoid double deregister". [1] line 23 on the "base". I would suggest you verify the sm_timer.h to see if it include the changes in [1], and possibly clean up your build environment to see if it helps. If your sm-common build also encountered the same errors, you may want to verify the size of int64_t on your build environment, it is expected to be 8-byte integer. Regards, Bin [1] https://review.opendev.org/#/c/675936/3/service-mgmt/sm-common-1.0.0/src/sm_... [2] https://review.opendev.org/#/c/675936/3/service-mgmt/sm-common-1.0.0/src/sm_... ________________________________________ From: Dominig ar Foll (Intel Open Source) [dominig.arfoll@fridu.net] Sent: Wednesday, September 25, 2019 3:10 AM To: starlingx-discuss@lists.starlingx.io Subject: Re: [Starlingx-discuss] openSUSE: sm-db build failure OpenSUSE is pretty strict when it comes to the quality of the code which gets in their repo and enforce some compiler warnings to to upgraded as error. There are several ways to correct those issues: 1) fix the problem in the source The best and prefer option is to fix the code. These type errors can induce nasty side effect and often are easy to correct. 2) change the compiler option In the make file, select the the required option to ignore the error. If there is no warning from the compiler, OpenSUSE will assume (rightly or wrongly) that you know what you do and you had no choose in keeping such ugliness. 3) use rpmlinrc rpmlint calculate an ugliness marking and if you pass the threshold it fails the package. You can change the ugliness marks allocated to certain type of errors. It's often easier to go around the issue, when possible (not for all warning type) but as bad as solution 2). Conclusion. ------------------ It's often not longer to fix the code once for all. That is the prefer route. Dominig On 25/09/2019 02:55, Arce Moreno, Abraham wrote:
Hi Bin,
Based in our work for openSUSE [0], and due to rpmlint configuration, we are having 2 warnings treated as errors while building sm-db package:
[ 4s] sm_db_service_heartbeat.c: In function 'SmErrorT sm_db_service_heartbeat_insert(SmDbHandleT*, SmDbServiceHeartbeatT*)': [ 4s] sm_db_service_heartbeat.c:321:13: error: format '%li' expects argument of type 'long int', but argument 35 has type 'SmTimerIdT {aka int}' [-Werror=format=] [ 4s] record->heartbeat_timer_id, [ 4s] sm_db_service_heartbeat.c: In function 'SmErrorT sm_db_service_heartbeat_update(SmDbHandleT*, SmDbServiceHeartbeatT*)': [ 4s] sm_db_service_heartbeat.c:438:49: error: format '%li' expects argument of type 'long int', but argument 5 has type 'SmTimerIdT {aka int}' [-Werror=format=] [ 4s] record->heartbeat_timer_id ); [ 4s] cc1plus: all warnings being treated as errors
We have confirmed the package gets built after reverting commit "Enhance timer system to avoid double deregister" [1]. Can you please guide us into the right direction to get this properly fixed?
[0] https://review.opendev.org/#/c/679686/ [1] https://review.opendev.org/#/c/675936/
_______________________________________________ Starlingx-discuss mailing list Starlingx-discuss@lists.starlingx.io http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
-- Dominig ar Foll Senior Software Architect Intel Open Source Technology Centre _______________________________________________ Starlingx-discuss mailing list Starlingx-discuss@lists.starlingx.io http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
Thanks Bin,
I am wondering there is something not right in your build environment. The SmTimerIdT is defined as: typedef int64_t SmTimerIdT; in sm_timer.h [1] patch set 1..3, which is part of the sm-common lib. There are multiple printf statements in sm-common lib that use the formater "%li" such as [2], line 150, 201. I don't expect you see the same errors from sm-common build, or otherwise the sm-db won't start building as it needs sm-common.
I am using container and virtual machine (vagrant) as development environments in a Ubuntu workstation.
Based on the error message in your first email, apparently the SmTimerIdT was "int" ('SmTimerIdT {aka int}'). That was the code before my commit "Enhance timer system to avoid double deregister". [1] line 23 on the "base".
Let me check again, I have repeated the process to confirm I am really using upstream code with our openSUSE HA specfiles.
I would suggest you verify the sm_timer.h to see if it include the changes in [1], and possibly clean up your build environment to see if it helps. If your sm-common build also encountered the same errors, you may want to verify the size of int64_t on your build environment, it is expected to be 8- byte integer.
I will... thanks!
Please DO NOT remove the compile flag -Werror. In this case it could cause read buffer overflow, as the printf will read 8 bytes from a 4 bytes allocated memory address, which could be significant stability and security issue. We will have to identify and solve the issue of a broken link SmTimerIdT = int64_t = long long int = 8 bytes integer. Thanks, Bin ________________________________________ From: Arce Moreno, Abraham [abraham.arce.moreno@intel.com] Sent: Wednesday, September 25, 2019 9:02 AM To: Qian, Bin; Dominig ar Foll (Intel Open Source); starlingx-discuss@lists.starlingx.io Subject: RE: [Starlingx-discuss] openSUSE: sm-db build failure Thanks Bin,
I am wondering there is something not right in your build environment. The SmTimerIdT is defined as: typedef int64_t SmTimerIdT; in sm_timer.h [1] patch set 1..3, which is part of the sm-common lib. There are multiple printf statements in sm-common lib that use the formater "%li" such as [2], line 150, 201. I don't expect you see the same errors from sm-common build, or otherwise the sm-db won't start building as it needs sm-common.
I am using container and virtual machine (vagrant) as development environments in a Ubuntu workstation.
Based on the error message in your first email, apparently the SmTimerIdT was "int" ('SmTimerIdT {aka int}'). That was the code before my commit "Enhance timer system to avoid double deregister". [1] line 23 on the "base".
Let me check again, I have repeated the process to confirm I am really using upstream code with our openSUSE HA specfiles.
I would suggest you verify the sm_timer.h to see if it include the changes in [1], and possibly clean up your build environment to see if it helps. If your sm-common build also encountered the same errors, you may want to verify the size of int64_t on your build environment, it is expected to be 8- byte integer.
I will... thanks!
Thanks Dominig,
OpenSUSE is pretty strict when it comes to the quality of the code which gets in their repo and enforce some compiler warnings to to upgraded as error. There are several ways to correct those issues:
1) fix the problem in the source The best and prefer option is to fix the code. These type errors can induce nasty side effect and often are easy to correct.
2) change the compiler option In the make file, select the the required option to ignore the error. If there is no warning from the compiler, OpenSUSE will assume (rightly or wrongly) that you know what you do and you had no choose in keeping such ugliness.
Yes, if I remove -Werror flag under CCFLAGS in src/Makefile, rpm gets generated.
3) use rpmlinrc rpmlint calculate an ugliness marking and if you pass the threshold it fails the package. You can change the ugliness marks allocated to certain type of errors. It's often easier to go around the issue, when possible (not for all warning type) but as bad as solution 2).
I do not know about this tool, I will learn about it.
Conclusion. ------------------ It's often not longer to fix the code once for all. That is the prefer route.
Let me give second review to go for option one.
participants (3)
-
Arce Moreno, Abraham
-
Dominig ar Foll (Intel Open Source)
-
Qian, Bin