On Wed, Oct 18 2017, Ori Bernstein <o...@eigenstate.org> wrote:
> On Thu, 19 Oct 2017 00:22:51 +0200
> Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote:
>
>> Those variables are static so that error messages are only printed once
>> in the shell lifetime.  history reload happens each time the shell
>> detects that the histfile has been modified, which can happen often if
>> like me you have multiple interactives shells running at the same time.
>  
> Makes sense, although I had been thinking that it could lead to lost
> warnings when changing histfile; I'd hope that a corrupt histfile isn't
> a persistent condition.

That's a fair point, I think I understand your previous proposal better.

Warning only once per histfile would need something like a file-global
variable - that we should reset in hist_init.  This sounds like extra
quirkiness that would make the code more complicated for a limited
benefit.  In the end, it might eat some of the precious 24 lines of your
terminal, but you can also just fix the offending lines.

> In any case, this looks pretty good to me.

Thanks for the feedback.

>> > Maybe hoist this outside of the loop.
>> 
>> I don't see a reason for this.

(Now I see.)

[...]

Updated diff.  The logic has changed, hopefully giving more accurate
warning messages.

Feedback / oks welcome.


Index: history.c
===================================================================
RCS file: /d/cvs/src/bin/ksh/history.c,v
retrieving revision 1.72
diff -u -p -p -u -r1.72 history.c
--- history.c   18 Oct 2017 15:41:25 -0000      1.72
+++ history.c   19 Oct 2017 07:56:05 -0000
@@ -740,23 +740,38 @@ static void
 history_load(Source *s)
 {
        char            *p, encoded[LINE + 1], line[LINE + 1];
+       int              toolongseen = 0;
 
        rewind(histfh);
+       line_co = 1;
 
        /* just read it all; will auto resize history upon next command */
-       for (line_co = 1; ; line_co++) {
-               p = fgets(encoded, sizeof(encoded), histfh);
-               if (p == NULL || feof(histfh) || ferror(histfh))
-                       break;
+       while (fgets(encoded, sizeof(encoded), histfh)) {
                if ((p = strchr(encoded, '\n')) == NULL) {
-                       bi_errorf("history file is corrupt");
-                       return;
+                       /* discard overlong line */
+                       do {
+                               /* maybe a missing trailing newline? */
+                               if (strlen(encoded) != sizeof(encoded) - 1) {
+                                       bi_errorf("history file is corrupt");
+                                       return;
+                               }
+                       } while (fgets(encoded, sizeof(encoded), histfh)
+                           && strchr(encoded, '\n') == NULL);
+
+                       if (!toolongseen) {
+                               toolongseen = 1;
+                               bi_errorf("ignored history line(s) longer than"
+                                   " %d bytes", LINE);
+                       }
+
+                       continue;
                }
                *p = '\0';
                s->line = line_co;
                s->cmd_offset = line_co;
                strunvis(line, encoded);
                histsave(line_co, line, 0);
+               line_co++;
        }
 
        history_write();

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to