[Buildroot] [PATCH 1/1] apr: fix size of pid_t

[Buildroot] [PATCH 1/1] apr: fix size of pid_t

[Julien Beraud]

pid_t is a signed 32bits integer on both 32bits and 64bits
architectures.
This fixes an issue with apache server which causes bad pid
to be written in PidFile

Signed-off-by: Julien Beraud <julien.beraud@spectracom.orolia.com>
---
 package/apr/apr.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/apr/apr.mk b/package/apr/apr.mk
index 361a79f99..f09f94368 100644
--- a/package/apr/apr.mk
+++ b/package/apr/apr.mk
@@ -22,6 +22,7 @@ APR_CONF_ENV = \
 	apr_cv_mutex_robust_shared=no \
 	apr_cv_tcp_nodelay_with_cork=yes \
 	ac_cv_sizeof_struct_iovec=8 \
+	ac_cv_sizeof_pid_t=4 \
 	ac_cv_struct_rlimit=yes \
 	ac_cv_o_nonblock_inherited=no \
 	apr_cv_mutex_recursive=yes
-- 
2.11.0

[Buildroot] [PATCH 1/1] apr: fix size of pid_t

[Thomas Petazzoni]

Hello,

On Tue, 21 Mar 2017 14:23:56 +0100, Julien Beraud wrote:
> pid_t is a signed 32bits integer on both 32bits and 64bits
> architectures.
> This fixes an issue with apache server which causes bad pid
> to be written in PidFile
> 
> Signed-off-by: Julien Beraud <julien.beraud@spectracom.orolia.com>
> ---
>  package/apr/apr.mk | 1 +
>  1 file changed, 1 insertion(+)

I've applied to our master branch, thanks! Peter, I believe this is a
good candidate for the LTS branch.

However, a few comments:

 * It is strange that their configure script does:

   APR_CHECK_SIZEOF_EXTENDED([#include <sys/types.h>], pid_t, 8)

   which means "assume size is 8 bytes" if cross-compiling when pid_t
   is always 4 bytes.

 * There are other sizeof that are probably bogus:

   APR_CHECK_SIZEOF_EXTENDED([#include <sys/types.h>], ssize_t, 8)
   APR_CHECK_SIZEOF_EXTENDED([#include <stddef.h>], size_t, 8)
   APR_CHECK_SIZEOF_EXTENDED([#include <sys/types.h>], off_t, 8)

   Indeed on 32 bit systems, ssize_t, size_t are 4 bytes. off_t is an
   even more complicated beast: it's 8 bytes on 64 bits platform, but
   on 32 bits platform, it depends if large file support is enabled or
   not.

See (executed on x86-64) :

$ cat toto.c 
#include <unistd.h>
#include <sys/types.h>
#include <stdio.h>

int main(void)
{
	printf("pid_t = %ld\n", sizeof(pid_t));
	printf("size_t = %ld\n", sizeof(size_t));
	printf("ssize_t = %ld\n", sizeof(ssize_t));
	printf("off_t = %ld\n", sizeof(off_t));
	return 0;
}
$ gcc -o toto toto.c
$ ./toto
pid_t = 4
size_t = 8
ssize_t = 8
off_t = 8
$ gcc -m32 -o toto toto.c
$ ./toto 
pid_t = 4
size_t = 4
ssize_t = 4
off_t = 4
$ gcc -m32 -D_FILE_OFFSET_BITS=64 -o toto toto.c
$ ./toto 
pid_t = 4
size_t = 4
ssize_t = 4
off_t = 8

I really wonder why they are not using the regular AC_CHECK_SIZEOF()
for those types.

So I believe there's more stuff to fix in there.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] apr: fix size of pid_t

[Julien Béraud]

Thanks Thomas,

>Hello,
>
>On Tue, 21 Mar 2017 14:23:56 +0100, Julien Beraud wrote:
>> pid_t is a signed 32bits integer on both 32bits and 64bits 
>> architectures.
>> This fixes an issue with apache server which causes bad pid to be 
>> written in PidFile
>> 
>> Signed-off-by: Julien Beraud <julien.beraud@spectracom.orolia.com>
>> ---
>>  package/apr/apr.mk | 1 +
>>  1 file changed, 1 insertion(+)
>
>I've applied to our master branch, thanks! Peter, I believe this is a good candidate for the LTS branch.
>
>However, a few comments:
>
> * It is strange that their configure script does:
>
>   APR_CHECK_SIZEOF_EXTENDED([#include <sys/types.h>], pid_t, 8)
>
>   which means "assume size is 8 bytes" if cross-compiling when pid_t
>   is always 4 bytes.
>
> * There are other sizeof that are probably bogus:
>
>   APR_CHECK_SIZEOF_EXTENDED([#include <sys/types.h>], ssize_t, 8)
>   APR_CHECK_SIZEOF_EXTENDED([#include <stddef.h>], size_t, 8)
>   APR_CHECK_SIZEOF_EXTENDED([#include <sys/types.h>], off_t, 8)
>
>   Indeed on 32 bit systems, ssize_t, size_t are 4 bytes. off_t is an
>   even more complicated beast: it's 8 bytes on 64 bits platform, but
>   on 32 bits platform, it depends if large file support is enabled or
>   not.
>
>See (executed on x86-64) :
>
>$ cat toto.c
>#include <unistd.h>
>#include <sys/types.h>
>#include <stdio.h>
>
>int main(void)
>{
>	printf("pid_t = %ld\n", sizeof(pid_t));
>	printf("size_t = %ld\n", sizeof(size_t));
>	printf("ssize_t = %ld\n", sizeof(ssize_t));
>	printf("off_t = %ld\n", sizeof(off_t));
>	return 0;
>}
>$ gcc -o toto toto.c
>$ ./toto
>pid_t = 4
>size_t = 8
>ssize_t = 8
>off_t = 8
>$ gcc -m32 -o toto toto.c
>$ ./toto
>pid_t = 4
>size_t = 4
>ssize_t = 4
>off_t = 4
>$ gcc -m32 -D_FILE_OFFSET_BITS=64 -o toto toto.c $ ./toto pid_t = 4 size_t = 4 ssize_t = 4 off_t = 8
>
>I really wonder why they are not using the regular AC_CHECK_SIZEOF() for those types.
>
>So I believe there's more stuff to fix in there.

The pid_t type was causing a very easy to notice issue, but the other types sizes being wrong could as
well be causing some issues.
I will take a look at the other types sizes and add them in order to avoid any possible issue. I will also
take a look at proposing patches to apr for those.

Regards,
Julien

[Buildroot] [PATCH 1/1] apr: fix size of pid_t

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Hello,
 > On Tue, 21 Mar 2017 14:23:56 +0100, Julien Beraud wrote:
 >> pid_t is a signed 32bits integer on both 32bits and 64bits
 >> architectures.
 >> This fixes an issue with apache server which causes bad pid
 >> to be written in PidFile
 >> 
 >> Signed-off-by: Julien Beraud <julien.beraud@spectracom.orolia.com>
 >> ---
 >> package/apr/apr.mk | 1 +
 >> 1 file changed, 1 insertion(+)

 > I've applied to our master branch, thanks! Peter, I believe this is a
 > good candidate for the LTS branch.

Committed to 2017.02.x, thanks.

-- 
Bye, Peter Korsgaard