From 65e323d266e2ebb2e5f9a935cbaa0181b5395740 Mon Sep 17 00:00:00 2001 From: Matthew Flatt Date: Tue, 28 Oct 2014 09:24:42 -0600 Subject: [PATCH] make-logger: rescind optional callback argument The optional callback argument was added (by me) in f2d87085. This is a backward-incompatible change, but allowing an arbitrary callback on a logger now seems like an especially bad idea; forms like `log-error` otherwise work in constrained contexts, while an arbitrary callback function allows potentially untrusted code in those contexts. Meanwhile, the addition doesn't satisfactorily solve the original problem, since it intereferes with `log-level?` and similar filters. --- .../scribblings/reference/logging.scrbl | 11 ++--- .../racket-test/tests/racket/logger.rktl | 41 +------------------ racket/src/racket/src/error.c | 20 +-------- racket/src/racket/src/mzmark_type.inc | 2 - racket/src/racket/src/mzmarksrc.c | 1 - racket/src/racket/src/schpriv.h | 1 - 6 files changed, 6 insertions(+), 70 deletions(-) diff --git a/pkgs/racket-pkgs/racket-doc/scribblings/reference/logging.scrbl b/pkgs/racket-pkgs/racket-doc/scribblings/reference/logging.scrbl index 8db3d2045a..5dd3b5f4fa 100644 --- a/pkgs/racket-pkgs/racket-doc/scribblings/reference/logging.scrbl +++ b/pkgs/racket-pkgs/racket-doc/scribblings/reference/logging.scrbl @@ -84,18 +84,13 @@ otherwise.} @defproc[(make-logger [name (or/c symbol? #f) #f] - [parent (or/c logger? #f) #f] - [notify-callback (vector? . -> . any/c) #f]) + [parent (or/c logger? #f) #f]) logger?]{ Creates a new @tech{logger} with an optional name and parent. -If @racket[notify-callback] is provided, then it is called (under a -@tech{continuation barrier}) whenever an event is logged to the result -@tech{logger} or one of its descendants, but only if some @tech{log -receiver} is interested in the event in the same sense as -@racket[log-level?]. The event is not propagated to any @tech{log -receivers} until @racket[notify-callback] returns.} +@history[#:changed "6.1.1.3" @elem{Removed an optional argument to + specify a notification callback.}]} @defproc[(logger-name [logger logger?]) (or/c symbol? #f)]{ diff --git a/pkgs/racket-pkgs/racket-test/tests/racket/logger.rktl b/pkgs/racket-pkgs/racket-test/tests/racket/logger.rktl index 89459d313b..63102b2f6c 100644 --- a/pkgs/racket-pkgs/racket-test/tests/racket/logger.rktl +++ b/pkgs/racket-pkgs/racket-test/tests/racket/logger.rktl @@ -13,7 +13,7 @@ (test #f logger-name (make-logger)) -(arity-test make-logger 0 3) +(arity-test make-logger 0 2) ; -------------------- @@ -102,7 +102,7 @@ (let () (define root (make-logger)) - (define sub1 (make-logger 'sub1 root #f)) + (define sub1 (make-logger 'sub1 root)) (define sub2 (make-logger 'sub2 root)) (define sub3 (make-logger 'sub3 root)) (define sub4 (make-logger 'sub4 root)) @@ -141,43 +141,6 @@ (log-message sub4 'fatal "message" 'data) (test #f get)) -; -------------------- -;; notification callback: - -(let () - (define rt #f) - (define s1 #f) - (define root (make-logger #f #f (lambda (m) (set! rt m)))) - (define sub1 (make-logger #f root (lambda (m) (set! s1 m)))) - ;; no receivers: - (log-message sub1 'debug "message" 'data) - (test #f values rt) - (test #f values s1) - (define r (make-log-receiver root 'error)) - ;; still no receivers for 'debug: - (log-message root 'debug "message" 'data) - (test #f values rt) - (test #f values s1) - ;; receivers for 'error: - (log-message sub1 'error "message" 'data) - (test rt vector 'error "message" 'data #f) - (test s1 vector 'error "message" 'data #f) - (set! rt #f) - (set! s1 #f) - (log-message root 'fatal 'name "message2" 'data2) - (test rt vector 'fatal "name: message2" 'data2 'name) - (test #f values s1) - (define sub2 (make-logger 'sub2 root (lambda (m) (abort-current-continuation - (default-continuation-prompt-tag) - void)))) - (test 'aborted - call-with-continuation-prompt - (lambda () (log-message sub2 'fatal 'name "message2" 'data2)) - (default-continuation-prompt-tag) - (lambda (v) 'aborted)) - - (void)) - ; -------------------- (let () diff --git a/racket/src/racket/src/error.c b/racket/src/racket/src/error.c index d62036f285..26def9f471 100644 --- a/racket/src/racket/src/error.c +++ b/racket/src/racket/src/error.c @@ -727,7 +727,7 @@ void scheme_init_error(Scheme_Env *env) GLOBAL_NONCM_PRIM("exit", scheme_do_exit, 0, 1, env); GLOBAL_NONCM_PRIM("log-level?", log_level_p, 2, 2, env); GLOBAL_NONCM_PRIM("log-max-level", log_max_level, 1, 1, env); - GLOBAL_NONCM_PRIM("make-logger", make_logger, 0, 3, env); + GLOBAL_NONCM_PRIM("make-logger", make_logger, 0, 2, env); GLOBAL_NONCM_PRIM("make-log-receiver", make_log_reader, 2, -1, env); GLOBAL_PRIM_W_ARITY("log-message", log_message, 4, 6, env); @@ -3589,18 +3589,6 @@ void scheme_log_name_pfx_message(Scheme_Logger *logger, int level, Scheme_Object if (!name) name = logger->name; - /* run notification callbacks: */ - for (lo = logger; lo; lo = lo->parent) { - if (lo->callback) { - Scheme_Object *a[1]; - if (!msg) - msg = make_log_message(level, name, prefix_msg, buffer, len, data); - - a[0] = msg; - scheme_apply_multi(lo->callback, 1, a); - } - } - if (SCHEME_FALSEP(name)) name = NULL; @@ -3946,9 +3934,6 @@ make_logger(int argc, Scheme_Object *argv[]) scheme_wrong_contract("make-logger", "(or/c logger? #f)", 1, argc, argv); parent = (Scheme_Logger *)argv[1]; } - - if (argc > 2) - (void)scheme_check_proc_arity2("make-logger", 1, 2, argc, argv, 1); } else parent = NULL; } else @@ -3958,9 +3943,6 @@ make_logger(int argc, Scheme_Object *argv[]) (argc ? (SCHEME_FALSEP(argv[0]) ? NULL : argv[0]) : NULL)); - - if ((argc > 2) && SCHEME_TRUEP(argv[2])) - logger->callback = argv[2]; return (Scheme_Object *)logger; } diff --git a/racket/src/racket/src/mzmark_type.inc b/racket/src/racket/src/mzmark_type.inc index 1c2613f350..4e85c28584 100644 --- a/racket/src/racket/src/mzmark_type.inc +++ b/racket/src/racket/src/mzmark_type.inc @@ -3004,7 +3004,6 @@ static int mark_logger_MARK(void *p, struct NewGC *gc) { Scheme_Logger *l = (Scheme_Logger *)p; gcMARK2(l->name, gc); gcMARK2(l->parent, gc); - gcMARK2(l->callback, gc); gcMARK2(l->timestamp, gc); gcMARK2(l->syslog_level, gc); gcMARK2(l->stderr_level, gc); @@ -3017,7 +3016,6 @@ static int mark_logger_FIXUP(void *p, struct NewGC *gc) { Scheme_Logger *l = (Scheme_Logger *)p; gcFIXUP2(l->name, gc); gcFIXUP2(l->parent, gc); - gcFIXUP2(l->callback, gc); gcFIXUP2(l->timestamp, gc); gcFIXUP2(l->syslog_level, gc); gcFIXUP2(l->stderr_level, gc); diff --git a/racket/src/racket/src/mzmarksrc.c b/racket/src/racket/src/mzmarksrc.c index f3303c340e..35940cd777 100644 --- a/racket/src/racket/src/mzmarksrc.c +++ b/racket/src/racket/src/mzmarksrc.c @@ -1207,7 +1207,6 @@ mark_logger { Scheme_Logger *l = (Scheme_Logger *)p; gcMARK2(l->name, gc); gcMARK2(l->parent, gc); - gcMARK2(l->callback, gc); gcMARK2(l->timestamp, gc); gcMARK2(l->syslog_level, gc); gcMARK2(l->stderr_level, gc); diff --git a/racket/src/racket/src/schpriv.h b/racket/src/racket/src/schpriv.h index 951ffdfc93..86bd484bf8 100644 --- a/racket/src/racket/src/schpriv.h +++ b/racket/src/racket/src/schpriv.h @@ -3693,7 +3693,6 @@ struct Scheme_Logger { Scheme_Object *name; Scheme_Logger *parent; int want_level; - Scheme_Object *callback; intptr_t *timestamp, local_timestamp; /* determines when want_level is up-to-date */ Scheme_Object *syslog_level; /* (list* ... ) */ Scheme_Object *stderr_level;