acpid in 1.16.2 doesn't build on SLES 10.

[Denys Vlasenko]

On Tue, Jul 20, 2010 at 9:46 PM, Rob Landley <rob at landley.net> wrote:
> Yes, but _after_ the if(value==0) statement, we then have:
>
>        if (value <= 1 || value >= 30000)
>                value = def_val;
>
> I.E. if the value passed in was 0, the default value is assigned.
>
>> Hmmm, it should be a bug then, but I don't see where:
>>
>> int FAST_FUNC get_terminal_width_height(int fd, unsigned *width, unsigned
>> *height) {
>>         struct winsize win;
>>         int err;
>>
>>         win.ws_row = 0;
>>         win.ws_col = 0;
>>         err = ioctl(fd, TIOCGWINSZ, &win) != 0 || win.ws_row == 0;
>> ** we get err = 1 here, since win.ws_row == 0
>
> Ok.  But these next lines don't check that:
>
>>         if (height)
>>                 *height = wh_helper(win.ws_row, 24, "LINES", &err);
>>         if (width)
>>                 *width = wh_helper(win.ws_col, 80, "COLUMNS", &err);
>
> All they care about is that the pointers "height" and "width" are not null.
> (They're not dereferencing them, they're just checking that the calling
> function was querying that property.  If so we call wh_helper(), which will
> replace a 0 value with the default value.
>
> So regardless of what get_terminal_width_height() is returning, *width and
> *height get set to default values.

Yes. That's how it is meant to be. get_terminal_width_height()
will get you sane values (1 < val < 32000) regardless of errors,
thus users which call get_terminal_width_height() usually
do not need to do the sanitization themselves.

But here, users (ash and vi) also want to know whether
there was an error querying the size - if there was one,
they will ignore returned width and height of 80x24
and will try "ask terminal" trick instead.

> And since the response from the probe returns arbitrarily later (nothing waits
> for it, not even the 1/3 second timeout vi uses after an escape key), it will
> presumably continue on to use those values for some time, and reset them to
> the defaults every time it probes for them.

Look for yourself what vi does at the file load time,
the moment when usually there is no buffered
keyboard input:

static void edit_file(char *fn)
{
...
        IF_FEATURE_VI_ASK_TERMINAL(G.get_rowcol_error =)
query_screen_dimensions();
#if ENABLE_FEATURE_VI_ASK_TERMINAL
        if (G.get_rowcol_error /* TODO? && no input on stdin */) {
                uint64_t k;
                write1("\033[999;999H" "\033[6n");
                fflush_all();
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
here we sent the query
                k = read_key(STDIN_FILENO, readbuffer, /*timeout_ms:*/ 100);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
here's the read with 100 ms timeout
                if ((int32_t)k == KEYCODE_CURSOR_POS) {
                        uint32_t rc = (k >> 32);
                        columns = (rc & 0x7fff);
                        if (columns > MAX_SCR_COLS)
                                columns = MAX_SCR_COLS;
                        rows = ((rc >> 16) & 0x7fff);
                        if (rows > MAX_SCR_ROWS)
                                rows = MAX_SCR_ROWS;
                }
        }
#endif

(This is the fixed code from
http://busybox.net/downloads/fixes-1.17.0/busybox-1.17.0-vi.patch)


And lineedit.c does this:

        /* Now initialize things */
        previous_SIGWINCH_handler = signal(SIGWINCH, win_changed);
        win_changed(0); /* do initial resizing */
^^^^^^^^^^^^^^^^^^^^^^^
does S.unknown_width = get_terminal_width_height(...)
...
        /* Print out the command prompt, optionally ask where cursor is */
        parse_and_put_prompt(prompt);
        ask_terminal();
^^^^^^^^^^^^^^^^^^^^^^^
sends query if S.unknown_width != 0 and no user input is pending, sets
S.sent_ESC_br6n = 1


and query result is processed by normal keyboard handling loop:

                if ((int32_t)ic == KEYCODE_CURSOR_POS
                 && S.sent_ESC_br6n
                ) {
                        S.sent_ESC_br6n = 0;
                        if (cursor == 0) { /* otherwise it may be bogus */
                                int col = ((ic >> 32) & 0x7fff) - 1;
                                if (col > cmdedit_prmt_len) {
                                        cmdedit_x += (col - cmdedit_prmt_len);
                                        while (cmdedit_x >= cmdedit_termw) {
                                                cmdedit_x -= cmdedit_termw;
                                                cmdedit_y++;
                                        }
                                }
                        }
                        continue;
                }

As I see it, if get_terminal_width_height() reports error,
bith vi and ash should fall back to "ask terminal" thing.
If they don't, there is still a bug somewhere, but not where
you point out.

-- 
vda