[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: debian/patches review



At Wed, 7 Aug 2002 15:47:41 -0400,
Ben Collins wrote:
> > string2-pointer-arith.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	I tested sample programs (compiled with gcc-2.95, 3.0,
> > 			3.1) stated in #45824, #44491, #44697, but I can't
> > 			reproduce this problem without this patch.
> > 			More test is needed but IMHO it can be removed after
> > 			-13.
> > 	Status:		remove
> 
> This one stays unless upstream merges it. With -Wall or -pedantic (can't
> remember which) we get warnings from string2.h.

Bug#45824 says compiling below sample code with -Wpointer-arith
then we get warnings. Example code is:

	mfx@laetitia:~/tmp > cat test.c
	#include <string.h>
	int main()
	{
	  return 0;
	}
	
	mfx@laetitia:~/tmp > gcc -Wpointer-arith -O2 -c test.c
	In file included from /usr/include/string.h:346,
			 from test.c:1:
	/usr/include/bits/string2.h: In function `__strcpy_small':
	/usr/include/bits/string2.h:419: warning: pointer of type `void *' used in arithmetic
	/usr/include/bits/string2.h:427: warning: pointer of type `void *' used in arithmetic
	/usr/include/bits/string2.h:432: warning: pointer of type `void *' used in arithmetic
	/usr/include/bits/string2.h:437: warning: pointer of type `void *' used in arithmetic
	/usr/include/bits/string2.h:439: warning: pointer of type `void *' used in arithmetic
	/usr/include/bits/string2.h:444: warning: pointer of type `void *' used in arithmetic
	
	mfx@laetitia:~/tmp > gcc -v
	Reading specs from /usr/lib/gcc-lib/i386-linux/2.95.1/specs
	gcc version 2.95.1 19990816 (release)

I also retest with another code (with extracting string2.h code forcibly):

	#include <string.h>
	#include <bits/string2.h>
	int main()
	{
		char a[20], *b;
		strncpy(b, a, 15);
		memset(a, 0, 11);
		__memset_gc(a, 0, 11);
		return 0;
	}
	
	gotom@celesta:~/tmp> gcc -Wpointer-arith -D_STRING_ARCH_unaligned sample.c

However I got no warnings. Am I missing something?

> > db1-addon-enabler.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	db1 addon enabling patch. keep applying it.
> > 	Status:		keep
> 
> Remove this now. We've ditched db1 support, and this patch can go.
>
> > glibc2.2.6-nice.dpatch
> > 	2.2 CVS:	-
> > 	2.3 CVS:	-
> > 	Comment:	It's already commented out. BenC?
> > 	Status:		already removed
> 
> Remove this.
>
> > sparc64-got-fix.dpatch
> > 	2.2 CVS:	-
> > 	2.3 CVS:	-
> > 	Comment:	It's already commented out. BenC?
> > 	Status:		already removed
> 
> this one aswell.

OK. We keep them in cvs for the moment, but we remove this entry
from 0list later.

> > glibc22-m68k-compat.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	Should be merged within upstream if it's ok.
> > 	Status:		merge
> 
> No, this is Debian specific. It's used for compatibility with older
> glibc/kernels. Upstream wont merge it, and we can't get rid of it.

My misunderstand. In previous mail I wrote to Michael, I wonder we
keep the patch only for sysdeps/unix/sysv/linux/m68k/chown.c...

> > ldd.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	it's useful, so we should send to upstream.
> > 			I want to know upstream author's opinion.
> > 	Status:		merge
> 
> Upstrea wont take this either. Mark it debian-specific.

Could you tell me which mail or BTS teach me the background of
this patch and the result of your discussion?
I still want to know the upstream opinion.

> > arm-no-hwcap.dpatch
> > 	2.2 CVS:	not in.
> > 	2.3 CVS:	not in.
> > 	Comment:	This patch only drops (HWCAP_ARM_HALF | HWCAP_ARM_FAST_MULT)
> > 			from HWCAP_IMPORTANT.
> > 			I couldn't understand why it was dropped. Ben?
> > 	Status:		unknown
> 
> Arm porters claimed the need for this. Email them for testing.

Could you tell me which BTS is refered about it?

> > glibc22-hppa-pthreads.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	? Ben?
> > 	Status:		unknown
> 
> Require for hppa. Should be merged upstream.
> 
> > glibc22-hppa-rela.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	? Ben?
> > 	Status:		unknown
> 
> Same here.

I try to send them to the upstream.

> > glibc22-nss-upgrade.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	? Ben?
> > 	Status:		unknown
> 
> This sticks around so that upgrades will be easier to 2.3, etc. Keep it
> around, and we'll go over it when the time comes.

OK, mark as 'for the upgrade'.

> > glibc22-ttyname-devfs.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	? I don't know it's ok or not.
> > 	Status:		unknown
> 
> This should be merged upstream, but I'm not sure they'll take it. If
> not, mark it debian-specific.

Well, we try to send them to the upstream.

> > manual-texinfo4.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	I don't know why it's changed... Debian specific ?
> > 	Status:		unknown
> 
> Debian specific. Our texinfo stuff is different than some others for
> some reason (maybe newer?).
>
> > nscd-security-fix.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	It seems it's debian specific patch, but IMHO,
> > 			we should discussed with upstream author.
> > 	Status:		unknown
> 
> This is Debian specific. It just defaults nscd.conf to disable host
> cache and explains why. Nothing about that needs to go upstream.
>
> > sparc64-fixups.dpatch
> > 	2.2 CVS:	not in
> > 	2.3 CVS:	not in
> > 	Comment:	I don't know about it. Why does elf/ldconfig.c have
> > 			architecture specific ifdef? Ben?
> > 	Status:		unknown
> 
> This patch keeps us from having to add /lib64 and /usr/lib64 to
> ld.so.conf. It needs to stay, just mark it Debian specific.

OK, change status as debian specific.
Thanks for your a lot of comments.

-- gotom



Reply to: