Date: Wed, 13 Oct 2010 01:11:10 -0700 From: Garrett Cooper <yanegomi@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-hackers@freebsd.org, Jilles Tjoelker <jilles@stack.nl> Subject: Re: [PATCH] Fix /bin/sh compilation with CFLAGS += -DDEBUG > 1 Message-ID: <AANLkTikZKZUUjVw1tt6Rg4s7=XMapdoQpEgEkt1J3=tr@mail.gmail.com> In-Reply-To: <201010121456.32819.jhb@freebsd.org> References: <AANLkTinHi6jaLY%2BbZdnhL=gEY3hWGrzcfFG8nO6VMc5n@mail.gmail.com> <201010120830.19872.jhb@freebsd.org> <AANLkTimq55BsTuKvFq7WHXKxtGk=Ny7L3JJ9fzOyOgDT@mail.gmail.com> <201010121456.32819.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On Tue, Oct 12, 2010 at 11:56 AM, John Baldwin <jhb@freebsd.org> wrote:
> On Tuesday, October 12, 2010 2:31:36 pm Garrett Cooper wrote:
>> On Tue, Oct 12, 2010 at 5:30 AM, John Baldwin <jhb@freebsd.org> wrote:
>> > You should use %z to print size_t's, but even better is to just use %t
>> > to print a ptrdiff_t (which is the type that holds the difference of two
>> > pointers).
>>
>> Ok. The overall temperature of using PRI* from POSIX seems like
>> it's undesirable; is it just POSIX cruft that FreeBSD conforms to in
>> theory only and doesn't really use in practice, or is there an example
>> of real practical application where it's used in the sourcebase?
>
> PRI* are ugly. FreeBSD provides it so that we are compliant and so that
> portable code can use it, but we do not use it in our source tree because
> it is unreadable.
Ok, I'll keep that in mind.
>> > The various changes in jobs.c should use '%td' as well rather than (int)
>> > casts.
>>
>> Ok. Tested build and runtime on amd64 and tested build-only with i386.
>
> Hmm, jobs.c shouldn't need any of the (ptrdiff_t) casts as the expression
> being printed is already a ptrdiff_t. See this non-debug code in jobs.c
> for example:
Good catch. Here's another patch minus the ptrdiff_t casts (again,
builds just fine on i386, and runs perfectly fine on amd64). As you
can probably tell, I haven't used the ptrdiff_t typedef before :).
Thanks!
-Garrett
[-- Attachment #2 --]
Index: bin/sh/Makefile
===================================================================
--- bin/sh/Makefile (revision 213680)
+++ bin/sh/Makefile (working copy)
@@ -21,7 +21,7 @@
LFLAGS= -8 # 8-bit lex scanner for arithmetic
CFLAGS+=-DSHELL -I. -I${.CURDIR}
# for debug:
-# CFLAGS+= -g -DDEBUG=2
+#DEBUG_FLAGS+= -g -DDEBUG=2
WARNS?= 2
WFORMAT=0
Index: bin/sh/expand.c
===================================================================
--- bin/sh/expand.c (revision 213680)
+++ bin/sh/expand.c (working copy)
@@ -43,14 +43,15 @@
#include <sys/types.h>
#include <sys/time.h>
#include <sys/stat.h>
+#include <dirent.h>
#include <errno.h>
-#include <dirent.h>
-#include <unistd.h>
+#include <inttypes.h>
+#include <limits.h>
#include <pwd.h>
+#include <stdio.h>
#include <stdlib.h>
-#include <limits.h>
-#include <stdio.h>
#include <string.h>
+#include <unistd.h>
/*
* Routines to expand arguments to commands. We have to deal with
@@ -497,9 +498,9 @@
exitstatus = waitforjob(in.jp, (int *)NULL);
if (quoted == 0)
recordregion(startloc, dest - stackblock(), 0);
- TRACE(("evalbackq: size=%d: \"%.*s\"\n",
- (dest - stackblock()) - startloc,
- (dest - stackblock()) - startloc,
+ TRACE(("expbackq: size=%td: \"%.*s\"\n",
+ ((dest - stackblock()) - startloc),
+ (int) ((dest - stackblock()) - startloc),
stackblock() + startloc));
expdest = dest;
INTON;
Index: bin/sh/jobs.c
===================================================================
--- bin/sh/jobs.c (revision 213680)
+++ bin/sh/jobs.c (working copy)
@@ -38,18 +38,18 @@
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
+#include <sys/ioctl.h>
+#include <sys/param.h>
+#include <sys/resource.h>
+#include <sys/stddef.h>
+#include <sys/time.h>
+#include <sys/wait.h>
+#include <errno.h>
#include <fcntl.h>
+#include <paths.h>
#include <signal.h>
-#include <errno.h>
-#include <paths.h>
+#include <stdlib.h>
#include <unistd.h>
-#include <stdlib.h>
-#include <sys/param.h>
-#include <sys/wait.h>
-#include <sys/time.h>
-#include <sys/resource.h>
-#include <paths.h>
-#include <sys/ioctl.h>
#include "shell.h"
#if JOBS
@@ -680,7 +680,7 @@
jp->ps = &jp->ps0;
}
INTON;
- TRACE(("makejob(%p, %d) returns %%%d\n", (void *)node, nprocs,
+ TRACE(("makejob(%p, %d) returns %%%td\n", (void *)node, nprocs,
jp - jobtab + 1));
return jp;
}
@@ -766,7 +766,7 @@
pid_t pid;
pid_t pgrp;
- TRACE(("forkshell(%%%d, %p, %d) called\n", jp - jobtab, (void *)n,
+ TRACE(("forkshell(%%%td, %p, %d) called\n", jp - jobtab, (void *)n,
mode));
INTOFF;
if (mode == FORK_BG)
@@ -903,7 +903,7 @@
int st;
INTOFF;
- TRACE(("waitforjob(%%%d) called\n", jp - jobtab + 1));
+ TRACE(("waitforjob(%%%td) called\n", jp - jobtab + 1));
while (jp->state == 0)
if (dowait(1, jp) == -1)
dotrap();
@@ -1004,7 +1004,7 @@
if (stopped) { /* stopped or done */
int state = done? JOBDONE : JOBSTOPPED;
if (jp->state != state) {
- TRACE(("Job %d: changing state from %d to %d\n", jp - jobtab + 1, jp->state, state));
+ TRACE(("Job %td: changing state from %d to %d\n", jp - jobtab + 1, jp->state, state));
jp->state = state;
if (jp != job) {
if (done && !jp->remembered &&
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTikZKZUUjVw1tt6Rg4s7=XMapdoQpEgEkt1J3=tr>