Date: Mon, 2 May 2011 09:24:58 -0700 From: Garrett Cooper <yanegomi@gmail.com> To: freebsd-hackers@freebsd.org Subject: Fwd: [PATCH] draft patch to make usr.bin/kdump WARNS?= 6 clean Message-ID: <BANLkTikHBVWkRDPBjSCkXLm-%2BUd-KisZgw@mail.gmail.com> In-Reply-To: <BANLkTimAZ3E6BNT606z_AN0uYGuVrN9N3A@mail.gmail.com> References: <BANLkTinR1qXERz9QJfneM4aKXhdLdz3ZtQ@mail.gmail.com> <BANLkTiniKkdQYs7vyWmoeYDjJMH%2B0oAFRg@mail.gmail.com> <BANLkTimAZ3E6BNT606z_AN0uYGuVrN9N3A@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
---------- Forwarded message ----------
From: Garrett Cooper <yanegomi@gmail.com>
Date: Mon, May 2, 2011 at 9:24 AM
Subject: Re: [PATCH] draft patch to make usr.bin/kdump WARNS?= 6 clean
To: Arnaud Lacombe <lacombar@gmail.com>
On Mon, May 2, 2011 at 9:21 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Mon, May 2, 2011 at 12:10 PM, Garrett Cooper <yanegomi@gmail.com> wrote:
>> I wanted to do something different this weekend, and I picked
>> usr.bin/kdump as a likely 'victim' for converting from WARNS?= 0 to
>> WARNS?= 6. I'm curious as to whether or not this is on the right
>> track, but here's the reasoning I used:
>>
>> 1. Conditionally include diskmbr.h or diskpc98.h based on whether or
>> not an architecture was non-pc98 or pc98 to avoid duplicate
>> definitions, because the beforementioned headers are mutually
>> exclusive.
>> 2. Move the sockfamilyname declaration to kdump_subr.h as it's used in
>> the generated ioctl.c file.
>> 3. Fix a signed vs unsigned comparison with a simple cast because the
>> size_t value will be sufficiently small that it can be converted to a
>> signed comparison.
>> 4. Fix a cast assignment type source//dest value alignment issue on
>> ia64 assigning a struct sockaddr value to either struct sockaddr_in or
>> struct sockaddr_in6 by using calloc and memcpy.
>> 5. Fix structure alignment issues on arm by marking some structures as __packed.
>> 6. Fix a shadowed declaration for flags by renaming a locally scoped
>> variable to _flags; add appropriate type to field.
>> 7. Remove unused argument to ktruser_malloc.
>> 8. Add missing declarations for ktruser_malloc and ktruser_rtld.
>>
>> I've run some basic tests and things seem sane (in particular
>> ktrace'ing ktrace :)... ktrace'ing 'ssh ::1' and ktrace'ing 'ssh
>> localhost', but I was wondering if there was anything I was missing or
>> if someone else who ran arm or ia64 could test this patch out for me.
>> I've run make universe on amd64, i386, ia64, mips, and pc98, and
>> things seem sane, but I can't play around with those machines to
>> determine whether or not they're functional at runtime with the above
>> changes.
>> Thanks!
>> -Garrett
>>
> I do not see any patch, either inline or attached.
>
> - Arnaud
>
>> PS Oh yeah... no commit bit means that I can't commit this either, but
>> I was curious if my approach was correct before getting to that step
>> :).
Yeah... I'm stupid for not attaching it. Need to get more sleep.
-Garrett
[-- Attachment #2 --]
Index: usr.bin/kdump/kdump.c
===================================================================
--- usr.bin/kdump/kdump.c (revision 221219)
+++ usr.bin/kdump/kdump.c (working copy)
@@ -99,8 +99,9 @@
void ktrsockaddr(struct sockaddr *);
void ktrstat(struct stat *);
void ktrstruct(char *, size_t);
+void ktruser_malloc(unsigned char *p);
+void ktruser_rtld(int len, unsigned char *p);
void usage(void);
-void sockfamilyname(int);
const char *ioctlname(u_long);
int timestamp, decimal, fancy = 1, suppressdata, tail, threads, maxdata,
@@ -528,13 +529,13 @@
ip++;
narg--;
} else if (ktr->ktr_code == SYS_open) {
- int flags;
+ u_int _flags;
int mode;
print_number(ip,narg,c);
- flags = *ip;
+ _flags = *ip;
mode = *++ip;
(void)putchar(',');
- flagsandmodename (flags, mode, decimal);
+ flagsandmodename (_flags, mode, decimal);
ip++;
narg-=2;
} else if (ktr->ktr_code == SYS_wait4) {
@@ -1256,7 +1257,7 @@
};
void
-ktruser_malloc(int len, unsigned char *p)
+ktruser_malloc(unsigned char *p)
{
struct utrace_malloc *ut = (struct utrace_malloc *)p;
@@ -1280,7 +1281,7 @@
}
if (len == sizeof(struct utrace_malloc)) {
- ktruser_malloc(len, p);
+ ktruser_malloc(p);
return;
}
@@ -1480,8 +1481,8 @@
if (datalen == 0)
goto invalid;
/* sanity check */
- for (i = 0; i < namelen; ++i)
- if (!isalpha((unsigned char)name[i]))
+ for (i = 0; i < (int)namelen; ++i)
+ if (!isalpha(name[i]))
goto invalid;
if (strcmp(name, "stat") == 0) {
if (datalen != sizeof(struct stat))
Index: usr.bin/kdump/kdump_subr.h
===================================================================
--- usr.bin/kdump/kdump_subr.h (revision 221219)
+++ usr.bin/kdump/kdump_subr.h (working copy)
@@ -45,3 +45,4 @@
void minheritname (int);
void quotactlname (int);
void ptraceopname (int);
+void sockfamilyname(int);
Index: usr.bin/kdump/mkioctls
===================================================================
--- usr.bin/kdump/mkioctls (revision 221219)
+++ usr.bin/kdump/mkioctls (working copy)
@@ -22,12 +22,21 @@
# XXX should we use an ANSI cpp?
ioctl_includes=`
cd $1
- find -H -s * -name '*.h' |
+ find -H -s * -name '*.h' -and \( -name ! 'diskmbr.h' -or -name ! 'diskpc98.h' \) |
xargs egrep -l \
'^#[ ]*define[ ]+[A-Za-z_][A-Za-z0-9_]*[ ]+_IO[^a-z0-9_]' |
awk '{printf("#include <%s>\\\\n", $1)}'
`
+case "`uname -m`" in
+*pc98*)
+ ioctl_includes="$ioctl_includes#include <sys/diskpc98.h>\\n"
+ ;;
+*)
+ ioctl_includes="$ioctl_includes#include <sys/diskmbr.h>\\n"
+ ;;
+esac
+
awk -v x="$ioctl_includes" 'BEGIN {print x}' |
gcc -E -I$1 -dM -DCOMPAT_43TTY - |
awk -v ioctl_includes="$ioctl_includes" -v use_switch="$use_switch" '
Index: usr.bin/kdump/Makefile
===================================================================
--- usr.bin/kdump/Makefile (revision 221219)
+++ usr.bin/kdump/Makefile (working copy)
@@ -15,7 +15,7 @@
SRCS+= linux_syscalls.c
.endif
-WARNS?= 0
+WARNS?= 6
CLEANFILES= ioctl.c kdump_subr.c linux_syscalls.c
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BANLkTikHBVWkRDPBjSCkXLm-%2BUd-KisZgw>