abort() is not portable -- it behaves differently on different systems.
A segfault is portable, so we use have used it for 20 years now, and it
works fine. Use abort() at your own risk.
On 3/4/19 8:35 AM, Gary R. Schmidt wrote:
> On 2019-03-01 23:00, Kern Sibbald wrote:
>> Hello,
>>
>> I am quite surprised that the static function did not take precedence.
>> In any case, I have changed the subroutine name to be bround(), and will
>> push it to the git rep this evening.
>>
>> Thanks for pointing this out,
>> Kern
>>
> I agree, it's strange that the "static" didn't hide it, but the G++
> developers often do strange things.
>
> Anyway, as threatened, here are my results building with the Solaris
> Developer Studio C++ compiler:
>
> Using Developer Studio 12.6.
>
> Firstly, the code compiles much more quietly than previously, the
> recent clean-up has certainly improved things.
>
> And the problem with "round()" did not happen...
>
> Sections are divided by lines of ='s.
>
> ==================================================================
> This warning is repeated for every file that includes src/config.h.
>
> Compiling attr.c
> /src/bacula/bacula-9.4.2/libtool --tag=CXX --mode=compile
> /opt/developerstudio12.6/bin/CC -c -I. -I.. -g -O -m64 attr.c
> libtool: compile: /opt/developerstudio12.6/bin/CC -c -I. -I.. -g -O
> -m64 attr.c -KPIC -DPIC -o .libs/attr.o
> "../config.h", line 1266: Warning (Anachronism): Attempt to redefine
> __restrict__ without using #undef.
> 1 Warning(s) detected.
>
> Examining the source, it is for a very old version of the Solaris C++
> compiler, I would suggest changing lines 1255-1266 in
> autoconf/config.h.in:
>
> $ diff -c original-bacula-9.4.2/autoconf/config.h.in
> bacula-9.4.2/autoconf/config.h.in
> *** original-bacula-9.4.2/autoconf/config.h.in Mon Mar 4 18:06:07 2019
> --- bacula-9.4.2/autoconf/config.h.in Mon Mar 4 18:11:18 2019
> ***************
> *** 1255,1266 ****
> nothing if this is not supported. Do not define if restrict is
> supported directly. */
> #undef restrict
> ! /* Work around a bug in Sun C++: it does not support _Restrict or
> __restrict__, even though the corresponding Sun C compiler ends
> up with
> "#define restrict _Restrict" or "#define restrict __restrict__"
> in the
> previous line. Perhaps some future version of Sun C++ will work
> with
> restrict; if so, hopefully it defines __RESTRICT like Sun C
> does. */
> ! #if defined __SUNPRO_CC && !defined __RESTRICT
> # define _Restrict
> # define __restrict__
> #endif
> --- 1255,1266 ----
> nothing if this is not supported. Do not define if restrict is
> supported directly. */
> #undef restrict
> ! /* Work around a bug in old Sun C++: it does not support _Restrict or
> __restrict__, even though the corresponding Sun C compiler ends
> up with
> "#define restrict _Restrict" or "#define restrict __restrict__"
> in the
> previous line. Perhaps some future version of Sun C++ will work
> with
> restrict; if so, hopefully it defines __RESTRICT like Sun C
> does. */
> ! #if defined __SUNPRO_CC && __SUNPRO_CC < 0x5150
> # define _Restrict
> # define __restrict__
> #endif
>
> This behaviour probably changed in Sun C++ several versions ago, but
> the noise generated prior to the recent clean-up masked it.
>
> ==================================================================
>
> After applying the above change, things get a lot quieter!
>
> Subsequent warnings fall into two basic groups.
>
> ==================================================================
>
> Group 1:
>
> "../baconfig.h", line 64: Warning: Likely null pointer dereference
> (*(((char *)0)[0])): bRC_BXATTR BXATTR::serialize_xattr_stream
>
> This looks like old C code used to cause a segmentation violation when
> something has gone wrong.
> I wonder why "abort()" was/is not used?
>
> What happens if you replace the assignments like "tjcr[0] = 0;" with
> "abort();"????
> The code compiles, but does it still do what is expected?? (Yes, I am
> sure that it does, and some of the lines are even documented with
> "generate segmentation violation" but just because I'm paranoid it
> doesn't mean they aren't out to get me!)
>
> There are three files affected:
> src/baconfig.h
> src/lib/lockmgr.c
> src/lib/message.c
>
> $ diff -c -r original-bacula-9.4.2/src/baconfig.h
> bacula-9.4.2/src/baconfig.h
> *** original-bacula-9.4.2/src/baconfig.h Tue Feb 5 03:47:31 2019
> --- bacula-9.4.2/src/baconfig.h Mon Mar 4 17:15:58 2019
> ***************
> *** 61,67 ****
> char *tjcr = NULL; \
> Emsg1(M_ERROR, 0, _("Failed ASSERT: %s\n"), #x); \
> Pmsg1(000, _("Failed ASSERT: %s\n"), #x); \
> ! tjcr[0] = 0; }
>
> #define ASSERT2(x,y) if (!(x)) { \
> set_assert_msg(__FILE__, __LINE__, y); \
> --- 61,67 ----
> char *tjcr = NULL; \
> Emsg1(M_ERROR, 0, _("Failed ASSERT: %s\n"), #x); \
> Pmsg1(000, _("Failed ASSERT: %s\n"), #x); \
> ! abort(); }
>
> #define ASSERT2(x,y) if (!(x)) { \
> set_assert_msg(__FILE__, __LINE__, y); \
> ***************
> *** 68,74 ****
> Emsg1(M_ERROR, 0, _("Failed ASSERT: %s\n"), #x); \
> Pmsg1(000, _("Failed ASSERT: %s\n"), #x); \
> char *tjcr = NULL; \
> ! tjcr[0] = 0; }
> #else
> #define ASSERT(x)
> #define ASSERT2(x, y)
> --- 68,74 ----
> Emsg1(M_ERROR, 0, _("Failed ASSERT: %s\n"), #x); \
> Pmsg1(000, _("Failed ASSERT: %s\n"), #x); \
> char *tjcr = NULL; \
> ! abort(); }
> #else
> #define ASSERT(x)
> #define ASSERT2(x, y)
>
> $ diff -c -r original-bacula-9.4.2/src/lib/lockmgr.c
> bacula-9.4.2/src/lib/lockmgr.c
> *** original-bacula-9.4.2/src/lib/lockmgr.c Tue Feb 5 03:47:31 2019
> --- bacula-9.4.2/src/lib/lockmgr.c Mon Mar 4 17:19:06 2019
> ***************
> *** 53,70 ****
> #define ASSERT(x) if (!(x)) { \
> char *jcr = NULL; \
> Pmsg3(000, _("ASSERT failed at %s:%i: %s\n"), __FILE__, __LINE__,
> #x); \
> ! jcr[0] = 0; }
>
> #define ASSERT_p(x,f,l) if (!(x)) { \
> char *jcr = NULL; \
> Pmsg3(000, _("ASSERT failed at %s:%i: %s \n"), f, l, #x); \
> ! jcr[0] = 0; }
>
> #define ASSERT2_p(x,m,f,l) if (!(x)) { \
> char *jcr = NULL; \
> set_assert_msg(f, l, m); \
> Pmsg4(000, _("ASSERT failed at %s:%i: %s (%s)\n"), f, l, #x,
> m); \
> ! jcr[0] = 0; }
>
> /* for lockmgr unit tests we have to clean up developer flags and
> asserts which breaks our tests */
> #ifdef TEST_PROGRAM
> --- 53,70 ----
> #define ASSERT(x) if (!(x)) { \
> char *jcr = NULL; \
> Pmsg3(000, _("ASSERT failed at %s:%i: %s\n"), __FILE__, __LINE__,
> #x); \
> ! abort(); }
>
> #define ASSERT_p(x,f,l) if (!(x)) { \
> char *jcr = NULL; \
> Pmsg3(000, _("ASSERT failed at %s:%i: %s \n"), f, l, #x); \
> ! abort(); }
>
> #define ASSERT2_p(x,m,f,l) if (!(x)) { \
> char *jcr = NULL; \
> set_assert_msg(f, l, m); \
> Pmsg4(000, _("ASSERT failed at %s:%i: %s (%s)\n"), f, l, #x,
> m); \
> ! abort(); }
>
> /* for lockmgr unit tests we have to clean up developer flags and
> asserts which breaks our tests */
> #ifdef TEST_PROGRAM
>
> $ diff -c -r original-bacula-9.4.2/src/lib/message.c
> bacula-9.4.2/src/lib/message.c
> *** original-bacula-9.4.2/src/lib/message.c Tue Feb 5 03:47:31 2019
> --- bacula-9.4.2/src/lib/message.c Mon Mar 4 17:17:12 2019
> ***************
> *** 1413,1419 ****
>
> if (type == M_ABORT) {
> char *p = 0;
> ! p[0] = 0; /* generate segmentation
> violation */
> }
> if (type == M_ERROR_TERM) {
> exit(1);
> --- 1413,1419 ----
>
> if (type == M_ABORT) {
> char *p = 0;
> ! abort(); /* generate segmentation
> violation */
> }
> if (type == M_ERROR_TERM) {
> exit(1);
> ***************
> *** 1551,1557 ****
> char *p = 0;
> printf("Bacula forced SEG FAULT to obtain traceback.\n");
> syslog(LOG_DAEMON|LOG_ERR, "Bacula forced SEG FAULT to obtain
> traceback.\n");
> ! p[0] = 0; /* generate segmentation
> violation */
> }
> if (type == M_ERROR_TERM) {
> exit(1);
> --- 1551,1557 ----
> char *p = 0;
> printf("Bacula forced SEG FAULT to obtain traceback.\n");
> syslog(LOG_DAEMON|LOG_ERR, "Bacula forced SEG FAULT to obtain
> traceback.\n");
> ! abort(); /* generate segmentation
> violation */
> }
> if (type == M_ERROR_TERM) {
> exit(1);
>
> ==================================================================
>
> Group 2:
>
> The bane of C++ coding, what happens when you have declarations mixed
> in with definitions?
>
> "../lib/cmd_parser.h", line 92: Warning: args hides cmd_parser::args.
> "../stored/aligned_dev.h", line 139: Warning:
> aligned_dev::get_full_addr hides the virtual function
> DEVICE::get_full_addr(unsigned, unsigned).
>
> The G++ developers appear to consider this benign, which it is,
> except, of course, when it isn't.
>
> These warnings can be switched off by adding
> "-erroff=wvarhidemem,hidevf" to CFLAGS, but I don't like to do that.
>
> ==================================================================
>
> Of the above three sets of problems, the __SUNPRO_CC guard code can be
> safely changed:
> #if defined __SUNPRO_CC && __SUNPRO_CC < 0x5150
>
> I am confident that using abort() will not change the behaviour of the
> code, but I'll let that mull for while, and see what Kern et al think.
>
> I dislike hiding warnings or errors, so I will not be doing this.
>
> Cheers,
> Gary B-)
>
>
>
> _______________________________________________
> Bacula-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/bacula-devel
_______________________________________________
Bacula-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bacula-devel
