Date: Fri, 2 Jun 2006 06:27:46 GMT From: John Birrell <jb@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 98309 for review Message-ID: <200606020627.k526RkgD044243@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=98309 Change 98309 by jb@jb_freebsd2 on 2006/06/02 06:27:00 DTrace's freopen() is problematic on FreeBSD because the "sematic cesspool that is standard I/O" is a different cesspool on FreeBSD. Ugh. The sematics of DTrace's freopen() function in the D language is that it allows output to be redirected to another file and then, later, the original file destination to be re-activated by doing a freopen(""). This is a problem on FreeBSD beacuse FreeBSD's freopen() tries to re-use the same file descriptor number. If the output was being sent to stdout, for instance, executing the Solaris dtrace_freopen() code would cause the original stdout destination to be closed and the new file opened as fd = 1. This is a good example of where Sun's DTrace code is well documented at the code level, but the big picture isn't clear. At least not to me. I can't tell if there is some other use for the output file descriptor that requires libdtrace to royally mess with the caller's file pointer like the Solaris code does. I have a feeling I'm going to get yelled at. 8-) OK, with that disclaimer on the record, this change is intended to achieve the same semantics as far as I understand them. I don't want to mess with the internals of file pointers or frig with the fact that FreeBSD doesn't like accessing /dev/fd/nn under some circumstances, so I'm going to use the ol' file pointer switcha-roo. If dtrace_freopen() is called, all it will do is either open an new file and save a pointer to the new file pointer structure; or close the current redirected file. And when dt_printf() is called, it will use the redirected file pointer if it is non-NULL instead of the caller's file pointer. This allows me to pass a grand total of one more test. Sigh. The ability to re-direct output is quite important to DTrace. Affected files ... .. //depot/projects/dtrace/src/contrib/opensolaris/lib/libdtrace/common/dt_impl.h#8 edit .. //depot/projects/dtrace/src/contrib/opensolaris/lib/libdtrace/common/dt_open.c#12 edit .. //depot/projects/dtrace/src/contrib/opensolaris/lib/libdtrace/common/dt_printf.c#5 edit .. //depot/projects/dtrace/src/contrib/opensolaris/lib/libdtrace/common/dt_subr.c#7 edit Differences ... ==== //depot/projects/dtrace/src/contrib/opensolaris/lib/libdtrace/common/dt_impl.h#8 (text) ==== @@ -275,7 +275,11 @@ int dt_fterr; /* saved errno from failed open of dt_ftfd */ int dt_cdefs_fd; /* file descriptor for C CTF debugging cache */ int dt_ddefs_fd; /* file descriptor for D CTF debugging cache */ +#if defined(sun) int dt_stdout_fd; /* file descriptor for saved stdout */ +#else + FILE *dt_freopen_fp; /* file pointer for freopened stdout */ +#endif dtrace_handle_err_f *dt_errhdlr; /* error handler, if any */ void *dt_errarg; /* error handler argument */ dtrace_prog_t *dt_errprog; /* error handler program, if any */ ==== //depot/projects/dtrace/src/contrib/opensolaris/lib/libdtrace/common/dt_open.c#12 (text) ==== @@ -995,7 +995,11 @@ dtp->dt_fterr = fterr; dtp->dt_cdefs_fd = -1; dtp->dt_ddefs_fd = -1; +#if defined(sun) dtp->dt_stdout_fd = -1; +#else + dtp->dt_freopen_fp = NULL; +#endif dtp->dt_modbuckets = _dtrace_strbuckets; dtp->dt_mods = calloc(dtp->dt_modbuckets, sizeof (dt_module_t *)); dtp->dt_provbuckets = _dtrace_strbuckets; @@ -1451,8 +1455,13 @@ (void) close(dtp->dt_cdefs_fd); if (dtp->dt_ddefs_fd != -1) (void) close(dtp->dt_ddefs_fd); +#if defined(sun) if (dtp->dt_stdout_fd != -1) (void) close(dtp->dt_stdout_fd); +#else + if (dtp->dt_freopen_fp != NULL) + (void) fclose(dtp->dt_freopen_fp); +#endif dt_epid_destroy(dtp); dt_aggid_destroy(dtp); ==== //depot/projects/dtrace/src/contrib/opensolaris/lib/libdtrace/common/dt_printf.c#5 (text) ==== @@ -1564,6 +1564,7 @@ if (rval == -1 || fp == NULL) return (rval); +#if defined(sun) if (pfd->pfd_preflen != 0 && strcmp(pfd->pfd_prefix, DT_FREOPEN_RESTORE) == 0) { /* @@ -1645,6 +1646,82 @@ } (void) fclose(nfp); +#else + /* + * The 'standard output' (which is not necessarily stdout) + * treatment on FreeBSD is implemented differently than on + * Solaris because FreeBSD's freopen() will attempt to re-use + * the current file descriptor, causing the previous file to + * be closed and thereby preventing it from be re-activated + * later. + * + * For FreeBSD we use the concept of setting an output file + * pointer in the DTrace handle if a dtrace_freopen() has + * enabled another output file and we leave the caller's + * file pointer untouched. If it was actually stdout, then + * stdout remains open. If it was another file, then that + * file remains open. While a dtrace_freopen() has activated + * another file, we keep a pointer to that which we use in + * the output functions by preference and only use the caller's + * file pointer if no dtrace_freopen() call has been made. + * + * The check to see if we're re-activating the caller's + * output file is much the same as on Solaris. + */ + if (pfd->pfd_preflen != 0 && + strcmp(pfd->pfd_prefix, DT_FREOPEN_RESTORE) == 0) { + /* + * The only way to have the format string set to the value + * DT_FREOPEN_RESTORE is via the empty freopen() string -- + * denoting that we should restore the old stdout. + */ + assert(strcmp(dtp->dt_sprintf_buf, DT_FREOPEN_RESTORE) == 0); + + if (dtp->dt_freopen_fp == NULL) { + /* + * We could complain here by generating an error, + * but it seems like overkill: it seems that calling + * freopen() to restore stdout when freopen() has + * never before been called should just be a no-op, + * so we just return in this case. + */ + return (rval); + } + + /* + * At this point, to re-active the original output file, + * on FreeBSD we only code the current file that this + * function opened previously. + */ + (void) fclose(dtp->dt_freopen_fp); + dtp->dt_freopen_fp = NULL; + + return (rval); + } + + if ((nfp = fopen(dtp->dt_sprintf_buf, "a")) == NULL) { + char *msg = strerror(errno); + char *faultstr; + int len = 80; + + len += strlen(msg) + strlen(dtp->dt_sprintf_buf); + faultstr = alloca(len); + + (void) snprintf(faultstr, len, "couldn't freopen() \"%s\": %s", + dtp->dt_sprintf_buf, strerror(errno)); + + if ((errval = dt_handle_liberr(dtp, data, faultstr)) == 0) + return (rval); + + return (errval); + } + + if (dtp->dt_freopen_fp != NULL) + (void) fclose(dtp->dt_freopen_fp); + + /* Remember that the output has been redirected to the new file. */ + dtp->dt_freopen_fp = nfp; +#endif return (rval); } ==== //depot/projects/dtrace/src/contrib/opensolaris/lib/libdtrace/common/dt_subr.c#7 (text) ==== @@ -562,6 +562,16 @@ va_list ap; int n; +#if !defined(sun) + /* + * On FreeBSD, check if output is currently being re-directed + * to another file. If so, output to that file instead of the + * one the caller has specified. + */ + if (dtp->dt_freopen_fp != NULL) + fp = dtp->dt_freopen_fp; +#endif + va_start(ap, format); if (dtp->dt_sprintf_buflen != 0) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200606020627.k526RkgD044243>