On Mon, May 18, 2009 at 7:08 AM, James Carlson <james.d.carl...@sun.com> wrote: > Jason King writes: >> I have an updated webrev http://cr.opensolaris.org/~jbk/ls-2 (in case >> anyone wants to compare to the original version). Hopefully, I've >> addressed all the points, but of course please let me know if I have >> not. >> >> I will send an email to psarc-ext a bit later with the two clarifications. > > One small issue in ls.c. At line 486 and 491, there's no guarantee > that scale_len is large enough to allow indexing to scale_len - 1 > scale_len - 2. Consider what will happen if the user does: > > --block-size= > > or: > > --block-size=B > > (For what it's worth, you could avoid all of the "if (si)" statements > if you had a "kilo" variable conditionally set to 1000 or 1024, and > just did "scale *= kilo".)
I'll have these fixed and update the 2nd webrev, for anyone that wants to look. > > Otherwise, your changes and your PSARC message both look good to me. One thing I realized I neglected to address in my last reply: the curses return codes. I'm not sure what the correct answer here is,the comments explicitly says it returns -1 in certain circumstances ( http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libcurses/screen/tiget.ed#72 ). It seems libxcurses and libxcurses2 does this as well. A quick search shows at least tput and prstat check for tigetstr returning -1 as well. So I'm not sure what the right answer here (I suspect there's probably some ancient history involved that I'm unaware of that would shed more light on things). I will at least (for clarity) move the check for -1 to after the tigetstr calls and simply set the lsc_* strings to NULL if tigetstr returns -1 (ls.c 3083-3090) instead of checking in ls_tprint(). _______________________________________________ opensolaris-code mailing list opensolaris-code@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/opensolaris-code
