On 07/26/2016 02:22 AM, Toni Spets wrote: > @hifi <https://github.com/hifi> pushed 1 commit. > > * 79ca172 <https://github.com/landley/toybox/commit/79ca172> Less > magic, more printf
It is so weird that github shows that commit as living in my tree instead of yours. I'm glad I only use that site for distribution of masters I keep elsewhere. (Is every commit in the whole of github in a single giant tree with a flat sha1hash collision namespace? I wonder how git catches that, or if the don't care until http://valerieaurora.org/hash.html goes red?) Ok, looking at git diff 7918d9ff8..79ca172eb0d0 The patch adds scr_init() allocating a screen buffer with 4 bytes per character... not an array of wide characters, but a char * with a width you multiply by 4. Ok? I have no idea how you're going to fit combining characters into there. You also don't appear to be storing color information, which is something less -R outputs... (So when you scroll the screen back up, you're not just outputting ESC[T and writing a new line of data at the top, but instead doing a memmove on your internal screen buffer? Why?) Next function: the patch adds scr_linediff()... which has no comment explaining what it does. It's doing a memcmp on an an array of char pointers... scr_init() allocated exacty 2 char pointers, where does this array with a length come from... Ok, let's jump down to the last function added to this file and see if it calls the earlier ones... scr_printf() is printing into SCR_PTR(). What's SCR_PTR? It's added to lib.h as #define SCR_PTR(scr) (scr->buf[0]) And this is the only user of that macro anywhere in the code so why is it a macro? If you try to write to >x or >y it returns with no error. Otherwise, this function is a wrapper around: vsnprintf(SCR_PTR(scr) + (scr->l * y), scr->l - x, fmt, va); That finds the start of the y line (scr->l is scr->w*4 and I don't know why you needed to save that to avoid a <<2 in the users), does _not_ advance x bytes (or bytes*4) into it, but truncates what we're printing to what presumably _would_ be the end of the line if we had advanced. Again, our screen width is *4 but we're not printing wide characters here. (Did you mean vswprintf?) ah, the second buffer _isn't_ to store color information, it's so you can diff what you think is on the screen (before the xterm was resized?) with what is now on the screen. So, next function up: scr_update() and THAT calls scr_linediff. Iterating through scr->h. What is scr->h? In lib.h it's "unsigned". Great. Back to look at scr_init()... Ah, it's screen height. Ok... + char *line = scr_linediff(scr->buf[1] + (scr->l * i), + scr->buf[0] + (scr->l * i), + scr->l, + i + 1); This is the only user of scr_linediff() in the code. Why it's a function, I couldn't tell you. Ah, I see! Those for() loops at the start of scr_linediff() are open-coding strnlen(). A non-wide-character version, which will stop at the first zero byte (which wide characters tend to be full of). And that's the chunk of time I had, I have a conference call in 12 minutes so need to go prepare for that... Rob _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
