Bug 10833 - lib/textpect/texpect.c requires signal.h
Summary: lib/textpect/texpect.c requires signal.h
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Other (show other bugs)
Version: unspecified
Hardware: All FreeBSD
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
: 11092 11148 (view as bug list)
Depends on:
Blocks: 10077
  Show dependency treegraph
 
Reported: 2014-09-24 11:55 UTC by Tristram Scott
Modified: 2015-03-15 20:57 UTC (History)
6 users (show)

See Also:


Attachments
Proposed patch for 4.2.0. (757 bytes, patch)
2015-03-02 23:09 UTC, Jeremy Allison
no flags Details
patchset of backports for 4.2 (6.34 KB, patch)
2015-03-02 23:43 UTC, Michael Adam
ddiss: review+
obnox: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tristram Scott 2014-09-24 11:55:33 UTC
Hi,

It seems that (at least on FreeBSD), texpect.c is missing an include for signal.h.  Perhaps this is implicit on other systems?

[ 694/3892] Compiling lib/talloc/pytalloc_util.c
[ 695/3892] Compiling lib/talloc/pytalloc.c
[ 696/3892] Compiling lib/texpect/texpect.c
../lib/texpect/texpect.c:79:1: error: unknown type name 'sig_atomic_t'
../lib/texpect/texpect.c: In function 'eval_parent':
../lib/texpect/texpect.c:265:20: error: 'SIGALRM' undeclared (first use in this function)
../lib/texpect/texpect.c:265:20: note: each undeclared identifier is reported only once for each function it appears in
../lib/texpect/texpect.c: In function 'main':
../lib/texpect/texpect.c:391:15: warning: assignment from incompatible pointer type [enabled by default]
../lib/texpect/texpect.c:430:22: error: storage size of 'sa' isn't known
../lib/texpect/texpect.c:436:15: error: 'SIGALRM' undeclared (first use in this function)
Waf: Leaving directory `/opt/src/samba42-gcc-5d22bcf/bin'
Build failed:  -> task failed (err #1): 
        {task: cc texpect.c -> texpect_1.o}
*** Error code 1

Stop.
make: stopped in /opt/src/samba42-gcc-5d22bcf
root@stor-pri-01:/opt/src/samba42-gcc-5d22bcf # uname -a
FreeBSD stor-pri-01.ds.lib.cam.ac.uk 10.0-RELEASE-p4 FreeBSD 10.0-RELEASE-p4 #0: Tue Jun  3 13:14:57 UTC 2014     root@amd64-builder.daemonology.net:/usr/obj/usr/src/sys/GENERIC  amd64
root@stor-pri-01:/opt/src/samba42-gcc-5d22bcf # 

This works for me, but I presume it would be better wrapped in an ifdef.

Regards,

Tristram
Comment 1 Tristram Scott 2014-09-24 12:18:40 UTC
Forgot to give branch info:

root@stor-pri-01:/opt/src/samba42-gcc-5d22bcf # git branch -v
* master 5d22bcf s3:torture: in LOCAL-MESSAGING-READ3, print some messages to child


Regards,

Tristram
Comment 2 Tom Schulz 2015-02-06 18:24:05 UTC
I ran into this while doing a test build of Samba 4.2.0rc4 on a Solaris 10 i386 machine. Solaris needs signal.h too. Fedora linux does not need it.
Comment 3 Tom Schulz 2015-02-10 16:10:32 UTC
Bug 11092 includes a patch that adds signal.h to texpect.c. Submitting two separate patches seemed too messy as the changer are right next to each other.
Comment 4 Jeremy Allison 2015-02-13 21:53:05 UTC
In master this got fixed by adding:

#include "system/filesys.h"
#include "system/wait.h"

after:

#include "replace.h"

in lib/texpect/texpect.c

Does that fix it in 4.2.x also ?
Comment 5 Tom Schulz 2015-02-17 20:33:07 UTC
The texpect.c in 4.2.0rc4 does not have

#include "replace.h"

in it, so I just added

#include "system/filesys.h"
#include "system/wait.h"

after the other includes in the same place that I had been adding

#ifdef HAVE_SIGNAL_H
#include <signal.h>
#endif

and it compiles correctly on Solaris 10. I don't have a FreeBSD system to test on.
Comment 6 Jeremy Allison 2015-03-02 23:09:00 UTC
Created attachment 10807 [details]
Proposed patch for 4.2.0.
Comment 7 Michael Adam 2015-03-02 23:20:21 UTC
Comment on attachment 10807 [details]
Proposed patch for 4.2.0.

This is part of commit 0f0148e020b6f85447f26de17c2b0b002bcdf498
from master. Shouldn't we possibly cherry-pick patches from master?
Comment 8 Michael Adam 2015-03-02 23:24:29 UTC
Here are the patches in master that are not in 4.2:

4bbfc54d09d813d1fb827de5855ce40e8eab1095 lib: texpect. Fix the build on Solaris.
0da7295fbc34170385d2b6bd165685aa092ab0ec lib/texpect: prefer bsd/libutil.h if available
e27a23e6aa27ae2a9b6ae2e2a2560943157aaa5c lib/texpect: fix compiler warnings
0f0148e020b6f85447f26de17c2b0b002bcdf498 lib/texpect: make the code more portable by using "replace.h" and "system/wait.h"
ccb0d9d6169594e8dd1c8935c9dfec51ee7125c4 lib/texpect: portability fix, include signal.h

most of these are portability related.
Should we pick all or most of them?
Comment 9 Jeremy Allison 2015-03-02 23:38:26 UTC
(In reply to Michael Adam from comment #7)

Don't think that cherry-pick will apply, I was just trying to limit changes at this stage. If you want to back port I'll +1 for 4.2.0.
Comment 10 Jeremy Allison 2015-03-02 23:38:46 UTC
(In reply to Michael Adam from comment #8)

Yeah that list looks safe enough to me.
Comment 11 Michael Adam 2015-03-02 23:43:44 UTC
Created attachment 10809 [details]
patchset of backports for 4.2

Patchset with backports of the relevant patches.
Untested hence not obsoleting original patchset for now.
Comment 12 Michael Adam 2015-03-02 23:52:14 UTC
Comment on attachment 10809 [details]
patchset of backports for 4.2

patchset compiles on linux for me.
can't test solaris/bsd today.
Comment 13 Tom Schulz 2015-03-03 16:38:46 UTC
The patchset compiles on Solaris 10 i386 and the texpect program displays the help message when invoked with --help.
Comment 14 Björn Jacke 2015-03-03 18:03:29 UTC
*** Bug 11092 has been marked as a duplicate of this bug. ***
Comment 15 Tom Schulz 2015-03-04 16:12:28 UTC
Is there still time to get this into 4.2.0 release?
Comment 16 Michael Adam 2015-03-04 16:46:59 UTC
Comment on attachment 10807 [details]
Proposed patch for 4.2.0.

obsoleted by more comprehensive patchset
Comment 17 Michael Adam 2015-03-04 16:47:30 UTC
either 4.2.0 or the next bugfix release
Comment 18 David Disseldorp 2015-03-04 17:45:44 UTC
Comment on attachment 10809 [details]
patchset of backports for 4.2

LGTM.
Comment 19 Michael Adam 2015-03-04 17:53:37 UTC
Comment on attachment 10809 [details]
patchset of backports for 4.2

explicit
Comment 20 David Disseldorp 2015-03-04 17:54:58 UTC
@Karolin, please merge for 4.2.
Comment 21 Karolin Seeger 2015-03-09 21:00:48 UTC
Pushed to autobuild-v4-2-test.
Comment 22 samba 2015-03-11 21:02:22 UTC
*** Bug 11148 has been marked as a duplicate of this bug. ***
Comment 23 samba 2015-03-11 21:03:30 UTC
Please note that this bug is still present in 4.2.0 release. Manually adding "#include <signal.h> to lib/texpect/texpect.c fixed the problem.
Comment 24 Michael Adam 2015-03-11 21:29:33 UTC
(In reply to samba from comment #23)

Right, the fix was too late for 4.2.0 final.
I it will definitely be included in the first
bugfix release 4.2.1.

Michael
Comment 25 Karolin Seeger 2015-03-15 20:57:37 UTC
Pushed to v4-2-test.
Will be included in 4.2.1.
Closing out bug report.

Thanks!