From 01dc58483866835b8d153d03070ec6edc138c5d5 Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Wed, 6 Jul 2011 15:56:32 -0400 Subject: [PATCH 1/7] Add truc_io:format and use it in the crash logger --- src/lager_crash_log.erl | 22 +++-------- src/trunc_io.erl | 88 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 89 insertions(+), 21 deletions(-) diff --git a/src/lager_crash_log.erl b/src/lager_crash_log.erl index 3c3a224..b8192df 100644 --- a/src/lager_crash_log.erl +++ b/src/lager_crash_log.erl @@ -69,12 +69,11 @@ handle_call(_Call, _From, State) -> %% @private handle_cast({log, Event}, #state{name=Name, fd=FD, inode=Inode, flap=Flap} = State) -> %% TODO these should probably be configurable and have saner defaults - FmtMaxBytes = 1024, - TermMaxSize = 500, + FmtMaxBytes = 6192, %% borrowed from riak_err {ReportStr, Pid, MsgStr, _ErrorP} = case Event of {error, _GL, {Pid1, Fmt, Args}} -> - {"ERROR REPORT", Pid1, limited_fmt(Fmt, Args, TermMaxSize, FmtMaxBytes), true}; + {"ERROR REPORT", Pid1, limited_fmt(Fmt, Args, FmtMaxBytes), true}; {error_report, _GL, {Pid1, std_error, Rep}} -> {"ERROR REPORT", Pid1, limited_str(Rep, FmtMaxBytes), true}; {error_report, _GL, Other} -> @@ -127,7 +126,7 @@ code_change(_OldVsn, State, _Extra) -> %% ===== Begin code lifted from riak_err ===== --spec limited_fmt(string(), list(), integer(), integer()) -> iolist(). +-spec limited_fmt(string(), list(), integer()) -> iolist(). %% @doc Format Fmt and Args similar to what io_lib:format/2 does but with %% limits on how large the formatted string may be. %% @@ -135,19 +134,8 @@ code_change(_OldVsn, State, _Extra) -> %% formatting is done by trunc_io:print/2, where FmtMaxBytes is used %% to limit the formatted string's size. -limited_fmt(Fmt, Args, TermMaxSize, FmtMaxBytes) -> - TermSize = erts_debug:flat_size(Args), - if TermSize > TermMaxSize -> - ["Oversize args for format \"", Fmt, "\": \n", - [ - begin - {Str, _} = trunc_io:print(lists:nth(N, Args), FmtMaxBytes), - [" arg", integer_to_list(N), ": ", Str, "\n"] - end || N <- lists:seq(1, length(Args)) - ]]; - true -> - io_lib:format(Fmt, Args) - end. +limited_fmt(Fmt, Args, FmtMaxBytes) -> + trunc_io:format(Fmt, Args, FmtMaxBytes). limited_str(Term, FmtMaxBytes) -> {Str, _} = trunc_io:print(Term, FmtMaxBytes), diff --git a/src/trunc_io.erl b/src/trunc_io.erl index a354385..2ddc433 100644 --- a/src/trunc_io.erl +++ b/src/trunc_io.erl @@ -29,10 +29,90 @@ -module(trunc_io). -author('matthias@corelatus.se'). %% And thanks to Chris Newcombe for a bug fix --export([print/2, fprint/2, safe/2]). % interface functions +-export([format/3, print/2, fprint/2, safe/2]). % interface functions -export([perf/0, perf/3, perf1/0, test/0, test/2]). % testing functions -version("$Id: trunc_io.erl,v 1.11 2009-02-23 12:01:06 matthias Exp $"). +format(String, Args, Max) -> + Parts = re:split(String, + "(~(?:-??\\d+\\.|\\*\\.|)(?:-??\d+\\.|\\*\\.|)(?:-??\\d+|\\*|)(?:t|)(?:[cfegswpWPBX#bx+ni]))", + [{return, list}, trim]), + Maxlen = Max - length(String), + format(Parts, Args, Maxlen, [], []). + +format([], _Args, _Max, Acc, ArgAcc) -> + io_lib:format(lists:flatten(lists:reverse(Acc)), lists:reverse(ArgAcc)); +format([[] | T], Args, Max, Acc, ArgAcc) -> + % discard the null list generated by split + format(T, Args, Max, Acc, ArgAcc); +format(["~n" | T], Args, Max, Acc, ArgAcc) -> + % ignore newlines for the purposes of argument indexing + format(T, Args, Max+1, ["~n" | Acc], ArgAcc); +format(["~i" | T], [AH | AT], Max, Acc, ArgAcc) -> + % ~i means ignore this argument, but we'll just pass it through + format(T, AT, Max+2, ["~i" | Acc], [AH | ArgAcc]); +format([[$~|H]| T], [AH1, AH2 | AT], Max, Acc, ArgAcc) when H == "W"; H == "P"; H == "X" -> + % these crazy ones consume TWO arguments, just pass them through + % because W and P implicitly limit size so we trust the user knows + % what they're doing. Unfortunately we can't change Max at all because + % depth based limits are not of known length. + format(T, AT, Max, [[$~ | H] | Acc], [AH2, AH1 | ArgAcc]); +format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) when length(H) == 1 -> + % single character format specifier, relatively simple + case H of + _ when H == "p"; H == "w"; H == "s" -> + %okay, these are prime candidates for rewriting + {String, Length} = case print(AH, Max) of + {_Res, Max} when AT /= [] -> + % this isn't the last argument, but it consumed all available space, give it half instead + print(AH, Max div 2); + {Res, Len} -> + {Res, Len} + end, + {Value, RealLen} = case H of + "s" -> + % strip off the doublequotes + {string:substr(String, 2, length(String) -2), Length -2}; + _ -> + {String, Length} + end, + format(T, AT, Max + 2 - RealLen, ["~s" | Acc], [Value | ArgAcc]); + _ -> + % whatever, just pass them on through + format(T, AT, Max, [[$~ | H], Acc], [AH | ArgAcc]) + end; +format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) -> + %% TODO + % complicated format specifier, gonna have to parse it.. + case re:run(H, "^(?:-??(\\d+|\\*)\\.|)(?:-??(\\d+|\\*)\\.|)(-??\\d+|\\*|)(t|)([cfegswpWPBX#bx+ni])$", [{capture, all_but_first, list}]) of + {match, [_F, _P, _Pad, _Mod, C]} when C == "p"; C=="w"; C=="s" -> + %okay, these are prime candidates for rewriting + {String, Length} = case print(AH, Max) of + {_Res, Max} when AT /= [] -> + % this isn't the last argument, but it consumed all available space, give it half instead + print(AH, Max div 2); + {Res, Len} -> + {Res, Len} + end, + {Value, RealLen} = case H of + "s" -> + % strip off the doublequotes + {string:substr(String, 2, length(String) -2), Length -2}; + _ -> + {String, Length} + end, + format(T, AT, Max + length(H) + 1 - RealLen, ["~s" | Acc], [Value | ArgAcc]); + {match, [_F, _P, _Pad, _Mod, C]} when C == "P"; C=="W" -> + [AH2 | AT2] = AT, + format(T, AT2, Max, [[$~|H]|Acc], [AH2, AH |ArgAcc]); + {match, _} -> + format(T, AT, Max, [H | Acc], [AH|ArgAcc]); + nomatch -> + io:format("unable to match format pattern ~~~p~n", [H]), + format(T, AT, Max, [H | Acc], [AH|ArgAcc]) + end; +format([H | T], Args, Max, Acc, ArgAcc) -> + format(T, Args, Max, [H | Acc], ArgAcc). %% @doc Returns an flattened list containing the ASCII representation of the given %% term. @@ -79,7 +159,7 @@ print(<<>>, _Max) -> print(Binary, Max) when is_binary(Binary) -> B = binary_to_list(Binary, 1, lists:min([Max, size(Binary)])), {L, Len} = alist_start(B, Max-4), - {["<<", L, ">>"], Len}; + {["<<", L, ">>"], Len+4}; print(Float, _Max) when is_float(Float) -> L = float_to_list(Float), @@ -146,7 +226,7 @@ alist_start([], _) -> {"[]", 2}; alist_start(_, Max) when Max < 4 -> {"...", 3}; alist_start([H|T], Max) when H >= 16#20, H =< 16#7e -> % definitely printable {L, Len} = alist([H|T], Max-1), - {[$\"|L], Len + 1}; + {[$"|L], Len + 1}; alist_start([H|T], Max) when H == 9; H == 10; H == 13 -> % show as space {L, Len} = alist(T, Max-1), {[$ |L], Len + 1}; @@ -164,7 +244,7 @@ alist([H|T], Max) when H == 9; H == 10; H == 13 -> % show as space {[$ |L], Len + 1}; alist(L, Max) -> {R, Len} = list_body(L, Max-3), - {[$\", $[, R, $]], Len + 3}. + {[$", $[, R, $]], Len + 3}. %%-------------------- From 37f190f03737263afe9d511404628110676a6729 Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Wed, 6 Jul 2011 19:08:15 -0400 Subject: [PATCH 2/7] Try to give more equal allocations to large terms --- src/trunc_io.erl | 76 +++++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 30 deletions(-) diff --git a/src/trunc_io.erl b/src/trunc_io.erl index 2ddc433..1a9d272 100644 --- a/src/trunc_io.erl +++ b/src/trunc_io.erl @@ -40,8 +40,9 @@ format(String, Args, Max) -> Maxlen = Max - length(String), format(Parts, Args, Maxlen, [], []). -format([], _Args, _Max, Acc, ArgAcc) -> - io_lib:format(lists:flatten(lists:reverse(Acc)), lists:reverse(ArgAcc)); +format([], _Args, Max, Acc, ArgAcc) -> + FmtArgs = resolve_futures(Max, ArgAcc), + io_lib:format(lists:flatten(lists:reverse(Acc)), lists:reverse(FmtArgs)); format([[] | T], Args, Max, Acc, ArgAcc) -> % discard the null list generated by split format(T, Args, Max, Acc, ArgAcc); @@ -62,21 +63,21 @@ format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) when length(H) == 1 -> case H of _ when H == "p"; H == "w"; H == "s" -> %okay, these are prime candidates for rewriting - {String, Length} = case print(AH, Max) of + case print(AH, Max) of {_Res, Max} when AT /= [] -> - % this isn't the last argument, but it consumed all available space, give it half instead - print(AH, Max div 2); - {Res, Len} -> - {Res, Len} - end, - {Value, RealLen} = case H of - "s" -> - % strip off the doublequotes - {string:substr(String, 2, length(String) -2), Length -2}; - _ -> - {String, Length} - end, - format(T, AT, Max + 2 - RealLen, ["~s" | Acc], [Value | ArgAcc]); + % this isn't the last argument, but it consumed all available space + % delay calculating the print size until the end + format(T, AT, Max + 2, ["~s" | Acc], [{future, AH} | ArgAcc]); + {String, Length} -> + {Value, RealLen} = case H of + "s" -> + % strip off the doublequotes + {string:substr(String, 2, length(String) -2), Length -2}; + _ -> + {String, Length} + end, + format(T, AT, Max + 2 - RealLen, ["~s" | Acc], [Value | ArgAcc]) + end; _ -> % whatever, just pass them on through format(T, AT, Max, [[$~ | H], Acc], [AH | ArgAcc]) @@ -87,22 +88,26 @@ format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) -> case re:run(H, "^(?:-??(\\d+|\\*)\\.|)(?:-??(\\d+|\\*)\\.|)(-??\\d+|\\*|)(t|)([cfegswpWPBX#bx+ni])$", [{capture, all_but_first, list}]) of {match, [_F, _P, _Pad, _Mod, C]} when C == "p"; C=="w"; C=="s" -> %okay, these are prime candidates for rewriting - {String, Length} = case print(AH, Max) of + case print(AH, Max) of {_Res, Max} when AT /= [] -> - % this isn't the last argument, but it consumed all available space, give it half instead - print(AH, Max div 2); - {Res, Len} -> - {Res, Len} - end, - {Value, RealLen} = case H of - "s" -> - % strip off the doublequotes - {string:substr(String, 2, length(String) -2), Length -2}; - _ -> - {String, Length} - end, - format(T, AT, Max + length(H) + 1 - RealLen, ["~s" | Acc], [Value | ArgAcc]); + % this isn't the last argument, but it consumed all available space + % delay calculating the print size until the end + format(T, AT, Max + length(H) + 1, ["~s" | Acc], [{future, AH} | ArgAcc]); + {String, Length} -> + {Value, RealLen} = case H of + "s" -> + % strip off the doublequotes + {string:substr(String, 2, length(String) -2), Length -2}; + _ -> + {String, Length} + end, + format(T, AT, Max + length(H) + 1 - RealLen, ["~s" | Acc], [Value | ArgAcc]) + end; {match, [_F, _P, _Pad, _Mod, C]} when C == "P"; C=="W" -> + % these crazy ones consume TWO arguments, just pass them through + % because W and P implicitly limit size so we trust the user knows + % what they're doing. Unfortunately we can't change Max at all because + % depth based limits are not of known length. [AH2 | AT2] = AT, format(T, AT2, Max, [[$~|H]|Acc], [AH2, AH |ArgAcc]); {match, _} -> @@ -114,6 +119,17 @@ format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) -> format([H | T], Args, Max, Acc, ArgAcc) -> format(T, Args, Max, [H | Acc], ArgAcc). +%% for all the really big terms encountered in a format/3 call, try to give each of them an equal share +resolve_futures(Max, Args) -> + Count = length(lists:filter(fun({future, _}) -> true; (_) -> false end, Args)), + case Count of + 0 -> + Args; + _ -> + SingleFmt = Max div Count, + lists:map(fun({future, Value}) -> element(1, print(Value, SingleFmt)); (X) -> X end, Args) + end. + %% @doc Returns an flattened list containing the ASCII representation of the given %% term. -spec fprint(term(), pos_integer()) -> string(). From f3b9fbd0fb8b7d1bb12c6fe1aacafc42046cabf2 Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Thu, 7 Jul 2011 13:11:33 -0400 Subject: [PATCH 3/7] Take the size of the format string into account when printing --- src/trunc_io.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/trunc_io.erl b/src/trunc_io.erl index 1a9d272..06e2972 100644 --- a/src/trunc_io.erl +++ b/src/trunc_io.erl @@ -63,7 +63,7 @@ format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) when length(H) == 1 -> case H of _ when H == "p"; H == "w"; H == "s" -> %okay, these are prime candidates for rewriting - case print(AH, Max) of + case print(AH, Max + 2) of {_Res, Max} when AT /= [] -> % this isn't the last argument, but it consumed all available space % delay calculating the print size until the end @@ -88,7 +88,7 @@ format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) -> case re:run(H, "^(?:-??(\\d+|\\*)\\.|)(?:-??(\\d+|\\*)\\.|)(-??\\d+|\\*|)(t|)([cfegswpWPBX#bx+ni])$", [{capture, all_but_first, list}]) of {match, [_F, _P, _Pad, _Mod, C]} when C == "p"; C=="w"; C=="s" -> %okay, these are prime candidates for rewriting - case print(AH, Max) of + case print(AH, Max + length(H) + 1) of {_Res, Max} when AT /= [] -> % this isn't the last argument, but it consumed all available space % delay calculating the print size until the end From c7ad3f80d5f63fe91f665deb55ec65e6f78ec424 Mon Sep 17 00:00:00 2001 From: Jon Meredith Date: Thu, 7 Jul 2011 11:55:53 -0600 Subject: [PATCH 4/7] Added basic eqc test for trunc_io:format with maxlen. --- test/trunc_io_eqc.erl | 157 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 test/trunc_io_eqc.erl diff --git a/test/trunc_io_eqc.erl b/test/trunc_io_eqc.erl new file mode 100644 index 0000000..549c483 --- /dev/null +++ b/test/trunc_io_eqc.erl @@ -0,0 +1,157 @@ +%% ------------------------------------------------------------------- +%% +%% trunc_io_eqc: QuickCheck test for trunc_io:format with maxlen +%% +%% Copyright (c) 2007-2011 Basho Technologies, Inc. All Rights Reserved. +%% +%% This file is provided to you under the Apache License, +%% Version 2.0 (the "License"); you may not use this file +%% except in compliance with the License. You may obtain +%% a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, +%% software distributed under the License is distributed on an +%% "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +%% KIND, either express or implied. See the License for the +%% specific language governing permissions and limitations +%% under the License. +%% +%% ------------------------------------------------------------------- +-module(trunc_io_eqc). + +-ifdef(TEST). +-ifdef(EQC). +-export([test/0, test/1, check/0]). + +-include_lib("eqc/include/eqc.hrl"). +-include_lib("eunit/include/eunit.hrl"). + +-define(QC_OUT(P), + eqc:on_output(fun(Str, Args) -> io:format(user, Str, Args) end, P)). + +%%==================================================================== +%% eunit test +%%==================================================================== + +eqc_test_() -> + {spawn, + [?_assertEqual(true, quickcheck(numtests(500, ?QC_OUT(prop_format()))))] + }. + +%%==================================================================== +%% Shell helpers +%%==================================================================== + +test() -> + test(100). + +test(N) -> + quickcheck(numtests(N, prop_format())). + +check() -> + check(prop_format(), current_counterexample()). + +%%==================================================================== +%% Generators +%%==================================================================== + +gen_fmt_args() -> + list(oneof([gen_print_str(), + {"~p", gen_any(5)}, + {"~w", gen_any(5)}, + {"~s", gen_print_str()} + ])). + + +%% Generates a printable string +gen_print_str() -> + ?LET(Xs, list(char()), [X || X <- Xs, io_lib:printable_list([X]), X /= $~]). + +gen_any(MaxDepth) -> + oneof([largeint(), + gen_atom(), + nat(), + binary(), + gen_pid(), + gen_port(), + gen_ref(), + gen_fun()] ++ + [?LAZY(list(gen_any(MaxDepth - 1))) || MaxDepth /= 0] ++ + [?LAZY(gen_tuple(gen_any(MaxDepth - 1))) || MaxDepth /= 0]). + +gen_atom() -> + elements([abc, def, ghi]). + +gen_tuple(Gen) -> + ?LET(Xs, list(Gen), list_to_tuple(Xs)). + +gen_max_len() -> %% Generate length from 3 to whatever. Needs space for ... in output + ?LET(Xs, int(), 3 + abs(Xs)). + +gen_pid() -> + ?LAZY(spawn(fun() -> ok end)). + +gen_port() -> + ?LAZY(begin + Port = erlang:open_port({spawn, "true"}, []), + erlang:port_close(Port), + Port + end). + +gen_ref() -> + ?LAZY(make_ref()). + +gen_fun() -> + ?LAZY(fun() -> ok end). + +%%==================================================================== +%% Property +%%==================================================================== + +%% Checks that trunc_io:format produces output less than or equal to MaxLen +prop_format() -> + ?FORALL({FmtArgs, MaxLen}, {gen_fmt_args(), gen_max_len()}, + begin + {FmtStr, Args} = build_fmt_args(FmtArgs), + try + Str = lists:flatten(trunc_io:format(FmtStr, Args, MaxLen)), + ?WHENFAIL(begin + io:format("FmtStr: ~p\n", [FmtStr]), + io:format("Args: ~p\n", [Args]), + io:format("MaxLen: ~p\n", [MaxLen]), + io:format("ActLen: ~p\n", [length(Str)]), + io:format("Str: ~p\n", [Str]) + end, + %% Make sure the result is a printable list + %% and if the format string is less than the length, + %% the result string is less than the length. + conjunction([{printable, Str == "" orelse + io_lib:printable_list(Str)}, + {length, length(FmtStr) > MaxLen orelse + length(Str) =< MaxLen}])) + catch + _:Err -> + io:format(user, "\nException: ~p\n", [Err]), + io:format("FmtStr: ~p\n", [FmtStr]), + io:format("Args: ~p\n", [Args]), + false + end + end). + +%%==================================================================== +%% Internal helpers +%%==================================================================== + +%% Build a tuple of {Fmt, Args} from a gen_fmt_args() return +build_fmt_args(FmtArgs) -> + F = fun({Fmt, Arg}, {FmtStr0, Args0}) -> + {FmtStr0 ++ Fmt, Args0 ++ [Arg]}; + (Str, {FmtStr0, Args0}) -> + {FmtStr0 ++ Str, Args0} + end, + lists:foldl(F, {"", []}, FmtArgs). + +-endif. % (TEST). +-endif. % (EQC). From 1f9ebb9e1ee3c1ca1b508f7a40077c756b442105 Mon Sep 17 00:00:00 2001 From: Jon Meredith Date: Thu, 7 Jul 2011 12:18:19 -0600 Subject: [PATCH 5/7] Added a FudgeLen to bound how far over it can go. trunc_io:format checks the length after formatting funs/bignums etc so it can go over. --- test/trunc_io_eqc.erl | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/test/trunc_io_eqc.erl b/test/trunc_io_eqc.erl index 549c483..85bd7c8 100644 --- a/test/trunc_io_eqc.erl +++ b/test/trunc_io_eqc.erl @@ -36,9 +36,10 @@ %%==================================================================== eqc_test_() -> - {spawn, - [?_assertEqual(true, quickcheck(numtests(500, ?QC_OUT(prop_format()))))] - }. + {timeout, 300, + {spawn, + [?_assertEqual(true, quickcheck(numtests(500, ?QC_OUT(prop_format()))))] + }}. %%==================================================================== %% Shell helpers @@ -114,15 +115,17 @@ gen_fun() -> prop_format() -> ?FORALL({FmtArgs, MaxLen}, {gen_fmt_args(), gen_max_len()}, begin + FudgeLen = 31, %% trunc_io does not correctly calc safe size of pid/port/numbers {FmtStr, Args} = build_fmt_args(FmtArgs), try Str = lists:flatten(trunc_io:format(FmtStr, Args, MaxLen)), ?WHENFAIL(begin - io:format("FmtStr: ~p\n", [FmtStr]), - io:format("Args: ~p\n", [Args]), - io:format("MaxLen: ~p\n", [MaxLen]), - io:format("ActLen: ~p\n", [length(Str)]), - io:format("Str: ~p\n", [Str]) + io:format(user, "FmtStr: ~p\n", [FmtStr]), + io:format(user, "Args: ~p\n", [Args]), + io:format(user, "FudgeLen: ~p\n", [FudgeLen]), + io:format(user, "MaxLen: ~p\n", [MaxLen]), + io:format(user, "ActLen: ~p\n", [length(Str)]), + io:format(user, "Str: ~p\n", [Str]) end, %% Make sure the result is a printable list %% and if the format string is less than the length, @@ -130,12 +133,12 @@ prop_format() -> conjunction([{printable, Str == "" orelse io_lib:printable_list(Str)}, {length, length(FmtStr) > MaxLen orelse - length(Str) =< MaxLen}])) + length(Str) =< MaxLen + FudgeLen}])) catch _:Err -> io:format(user, "\nException: ~p\n", [Err]), - io:format("FmtStr: ~p\n", [FmtStr]), - io:format("Args: ~p\n", [Args]), + io:format(user, "FmtStr: ~p\n", [FmtStr]), + io:format(user, "Args: ~p\n", [Args]), false end end). From 71fae1b89adf0fd818c53528fd28b6e4309508c3 Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Thu, 7 Jul 2011 14:57:49 -0400 Subject: [PATCH 6/7] Truncator for funs; special case binary truncation when max is 0 --- src/trunc_io.erl | 14 ++++++++++++-- test/trunc_io_eqc.erl | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/trunc_io.erl b/src/trunc_io.erl index 06e2972..23d4080 100644 --- a/src/trunc_io.erl +++ b/src/trunc_io.erl @@ -172,6 +172,9 @@ print(Atom, _Max) when is_atom(Atom) -> print(<<>>, _Max) -> {"<<>>", 4}; +print(Binary, 0) when is_binary(Binary) -> + {"<<..>>", 6}; + print(Binary, Max) when is_binary(Binary) -> B = binary_to_list(Binary, 1, lists:min([Max, size(Binary)])), {L, Len} = alist_start(B, Max-4), @@ -181,9 +184,16 @@ print(Float, _Max) when is_float(Float) -> L = float_to_list(Float), {L, length(L)}; -print(Fun, _Max) when is_function(Fun) -> +print(Fun, Max) when is_function(Fun) -> L = erlang:fun_to_list(Fun), - {L, length(L)}; + case length(L) > Max of + true -> + S = erlang:max(5, Max), + Res = string:substr(L, 1, S) ++ "..>", + {Res, length(Res)}; + _ -> + {L, length(L)} + end; print(Integer, _Max) when is_integer(Integer) -> L = integer_to_list(Integer), diff --git a/test/trunc_io_eqc.erl b/test/trunc_io_eqc.erl index 85bd7c8..4b96aab 100644 --- a/test/trunc_io_eqc.erl +++ b/test/trunc_io_eqc.erl @@ -115,7 +115,7 @@ gen_fun() -> prop_format() -> ?FORALL({FmtArgs, MaxLen}, {gen_fmt_args(), gen_max_len()}, begin - FudgeLen = 31, %% trunc_io does not correctly calc safe size of pid/port/numbers + FudgeLen = 31, %% trunc_io does not correctly calc safe size of pid/port/numbers/funs {FmtStr, Args} = build_fmt_args(FmtArgs), try Str = lists:flatten(trunc_io:format(FmtStr, Args, MaxLen)), From d230cbaa023576bb484b60714e985cb530e85a47 Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Thu, 7 Jul 2011 17:23:55 -0400 Subject: [PATCH 7/7] More fixes found by QC and some QC improvements --- src/trunc_io.erl | 73 +++++++++++++++++++++++++++++-------------- test/trunc_io_eqc.erl | 30 ++++++++++++++++-- 2 files changed, 78 insertions(+), 25 deletions(-) diff --git a/src/trunc_io.erl b/src/trunc_io.erl index 23d4080..288c384 100644 --- a/src/trunc_io.erl +++ b/src/trunc_io.erl @@ -35,7 +35,7 @@ format(String, Args, Max) -> Parts = re:split(String, - "(~(?:-??\\d+\\.|\\*\\.|)(?:-??\d+\\.|\\*\\.|)(?:-??\\d+|\\*|)(?:t|)(?:[cfegswpWPBX#bx+ni]))", + "(~(?:-??\\d+\\.|\\*\\.|\\.|)(?:-??\\d+\\.|\\*\\.|\\.|)(?:-??\\d+|\\*|)(?:t|)(?:[cfegswpWPBX#bx+ni~]))", [{return, list}, trim]), Maxlen = Max - length(String), format(Parts, Args, Maxlen, [], []). @@ -46,25 +46,37 @@ format([], _Args, Max, Acc, ArgAcc) -> format([[] | T], Args, Max, Acc, ArgAcc) -> % discard the null list generated by split format(T, Args, Max, Acc, ArgAcc); +format(["~~" | T], Args, Max, Acc, ArgAcc) -> + format(T, Args, Max+1, ["~~" | Acc], ArgAcc); format(["~n" | T], Args, Max, Acc, ArgAcc) -> % ignore newlines for the purposes of argument indexing format(T, Args, Max+1, ["~n" | Acc], ArgAcc); format(["~i" | T], [AH | AT], Max, Acc, ArgAcc) -> % ~i means ignore this argument, but we'll just pass it through format(T, AT, Max+2, ["~i" | Acc], [AH | ArgAcc]); -format([[$~|H]| T], [AH1, AH2 | AT], Max, Acc, ArgAcc) when H == "W"; H == "P"; H == "X" -> - % these crazy ones consume TWO arguments, just pass them through - % because W and P implicitly limit size so we trust the user knows - % what they're doing. Unfortunately we can't change Max at all because - % depth based limits are not of known length. - format(T, AT, Max, [[$~ | H] | Acc], [AH2, AH1 | ArgAcc]); +format([[$~|H]| T], [AH1, AH2 | AT], Max, Acc, ArgAcc) when H == "X"; H == "x" -> + %% ~X consumes 2 arguments. It only prints integers so we can leave it alone + format(T, AT, Max, ["~X" | Acc], [AH2, AH1 | ArgAcc]); +format([[$~|H]| T], [AH1, _AH2 | AT], Max, Acc, ArgAcc) when H == "W"; H == "P" -> + %% ~P and ~W consume 2 arguments, the second one being a depth limiter. + %% trunc_io isn't (yet) depth aware, so we can't honor this format string + %% safely at the moment, so just treat it like a regular ~p + %% TODO support for depth limiting + case print(AH1, Max + 2) of + {_Res, Max} -> + % this isn't the last argument, but it consumed all available space + % delay calculating the print size until the end + format(T, AT, Max + 2, ["~s" | Acc], [{future, AH1} | ArgAcc]); + {String, Length} -> + format(T, AT, Max + 2 - Length, ["~s" | Acc], [String | ArgAcc]) + end; format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) when length(H) == 1 -> % single character format specifier, relatively simple case H of _ when H == "p"; H == "w"; H == "s" -> %okay, these are prime candidates for rewriting case print(AH, Max + 2) of - {_Res, Max} when AT /= [] -> + {_Res, Max} -> % this isn't the last argument, but it consumed all available space % delay calculating the print size until the end format(T, AT, Max + 2, ["~s" | Acc], [{future, AH} | ArgAcc]); @@ -80,16 +92,18 @@ format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) when length(H) == 1 -> end; _ -> % whatever, just pass them on through - format(T, AT, Max, [[$~ | H], Acc], [AH | ArgAcc]) + format(T, AT, Max, [[$~ | H] | Acc], [AH | ArgAcc]) end; format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) -> - %% TODO % complicated format specifier, gonna have to parse it.. - case re:run(H, "^(?:-??(\\d+|\\*)\\.|)(?:-??(\\d+|\\*)\\.|)(-??\\d+|\\*|)(t|)([cfegswpWPBX#bx+ni])$", [{capture, all_but_first, list}]) of - {match, [_F, _P, _Pad, _Mod, C]} when C == "p"; C=="w"; C=="s" -> + %case re:run(H, "^(?:-??(\\d+|\\*)\\.|\\.|)(?:-??(\\d+|\\*)\\.|\\.|)(-??.|)(t|)([cfegswpWPBX#bx+ni])$", [{capture, all_but_first, list}]) of + %% its actually simpler to just look at the last character in the string + case lists:nth(length(H), H) of + C when C == $p; C == $w; C == $s -> + %{match, [_F, _P, _Pad, _Mod, C]} when C == "p"; C=="w"; C=="s" -> %okay, these are prime candidates for rewriting case print(AH, Max + length(H) + 1) of - {_Res, Max} when AT /= [] -> + {_Res, Max} -> % this isn't the last argument, but it consumed all available space % delay calculating the print size until the end format(T, AT, Max + length(H) + 1, ["~s" | Acc], [{future, AH} | ArgAcc]); @@ -103,18 +117,31 @@ format([[$~|H]| T], [AH | AT], Max, Acc, ArgAcc) -> end, format(T, AT, Max + length(H) + 1 - RealLen, ["~s" | Acc], [Value | ArgAcc]) end; - {match, [_F, _P, _Pad, _Mod, C]} when C == "P"; C=="W" -> - % these crazy ones consume TWO arguments, just pass them through - % because W and P implicitly limit size so we trust the user knows - % what they're doing. Unfortunately we can't change Max at all because - % depth based limits are not of known length. + C when C == $P; C == $W -> + %{match, [_F, _P, _Pad, _Mod, C]} when C == "P"; C == "W" -> + %% ~P and ~W consume 2 arguments, the second one being a depth limiter. + %% trunc_io isn't (yet) depth aware, so we can't honor this format string + %% safely at the moment, so just treat it like a regular ~p + %% TODO support for depth limiting + [_ | AT2] = AT, + case print(AH, Max + 2) of + {_Res, Max} -> + % this isn't the last argument, but it consumed all available space + % delay calculating the print size until the end + format(T, AT2, Max + 2, ["~s" | Acc], [{future, AH} | ArgAcc]); + {String, Length} -> + format(T, AT2, Max + 2 - Length, ["~s" | Acc], [String | ArgAcc]) + end; + C when C == $X; C == $x -> + %{match, [_F, _P, _Pad, _Mod, C]} when C == "X"; C == "x" -> + %% ~X consumes 2 arguments. It only prints integers so we can leave it alone [AH2 | AT2] = AT, format(T, AT2, Max, [[$~|H]|Acc], [AH2, AH |ArgAcc]); - {match, _} -> - format(T, AT, Max, [H | Acc], [AH|ArgAcc]); - nomatch -> - io:format("unable to match format pattern ~~~p~n", [H]), - format(T, AT, Max, [H | Acc], [AH|ArgAcc]) + _ -> + format(T, AT, Max, [[$~|H] | Acc], [AH|ArgAcc]) + %nomatch -> + %io:format("unable to match format pattern ~~~p~n", [H]), + %format(T, AT, Max, [[$~|H] | Acc], [AH|ArgAcc]) end; format([H | T], Args, Max, Acc, ArgAcc) -> format(T, Args, Max, [H | Acc], ArgAcc). diff --git a/test/trunc_io_eqc.erl b/test/trunc_io_eqc.erl index 4b96aab..f97830a 100644 --- a/test/trunc_io_eqc.erl +++ b/test/trunc_io_eqc.erl @@ -60,9 +60,29 @@ check() -> gen_fmt_args() -> list(oneof([gen_print_str(), + "~~", {"~p", gen_any(5)}, {"~w", gen_any(5)}, - {"~s", gen_print_str()} + {"~s", gen_print_str()}, + {"~P", gen_any(5), 4}, + {"~W", gen_any(5), 4}, + {"~i", gen_any(5)}, + {"~B", nat()}, + {"~b", nat()}, + {"~X", nat(), "0x"}, + {"~x", nat(), "0x"}, + {"~.10#", nat()}, + {"~.10+", nat()}, + {"~.36B", nat()}, + {"~62P", gen_any(5), 4}, + {"~c", gen_char()}, + {"~tc", gen_char()} + %{"~f", real()}, %% floats like to make the fudge limit fail, so don't enable them + %{"~10.f", real()}, + %{"~g", real()}, + %{"~10.g", real()}, + %{"~e", real()}, + %{"~10.e", real()} ])). @@ -74,6 +94,7 @@ gen_any(MaxDepth) -> oneof([largeint(), gen_atom(), nat(), + %real(), binary(), gen_pid(), gen_port(), @@ -106,6 +127,9 @@ gen_ref() -> gen_fun() -> ?LAZY(fun() -> ok end). + +gen_char() -> + oneof(lists:seq($A, $z)). %%==================================================================== %% Property @@ -115,7 +139,7 @@ gen_fun() -> prop_format() -> ?FORALL({FmtArgs, MaxLen}, {gen_fmt_args(), gen_max_len()}, begin - FudgeLen = 31, %% trunc_io does not correctly calc safe size of pid/port/numbers/funs + FudgeLen = 50, %% trunc_io does not correctly calc safe size of pid/port/numbers/funs {FmtStr, Args} = build_fmt_args(FmtArgs), try Str = lists:flatten(trunc_io:format(FmtStr, Args, MaxLen)), @@ -151,6 +175,8 @@ prop_format() -> build_fmt_args(FmtArgs) -> F = fun({Fmt, Arg}, {FmtStr0, Args0}) -> {FmtStr0 ++ Fmt, Args0 ++ [Arg]}; + ({Fmt, Arg1, Arg2}, {FmtStr0, Args0}) -> + {FmtStr0 ++ Fmt, Args0 ++ [Arg1, Arg2]}; (Str, {FmtStr0, Args0}) -> {FmtStr0 ++ Str, Args0} end,