Bug 210344

Summary: ul(1) truncates long lines
Product: Base System Reporter: Pietro Cerutti <gahr>
Component: binAssignee: Pietro Cerutti <gahr>
Status: Closed FIXED    
Severity: Affects Only Me CC: abhinav, gahr
Priority: --- Keywords: patch
Version: CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Accomodate long lines in ul(1)
none
Accomodate long lines in ul(1) - white space changes removed none

Description Pietro Cerutti freebsd_committer freebsd_triage 2016-06-17 14:09:00 UTC 
Created attachment 171507 [details]
Accomodate long lines in ul(1)

From this mailing list post:
http://lists.freebsd.org/pipermail/freebsd-hackers/2016-June/049596.html

ul(1) holds a static buffer of 512 characters and truncates any line longer than that.

The attached patch allows expanding the buffer to accommodate lines longer than 512 characters. This is only done on the first occurrence of such a line, so for most files no dynamic allocation is needed.

A version of the same patch with space changes ignored is also attached.
Comment 1 Pietro Cerutti freebsd_committer freebsd_triage 2016-06-17 14:09:28 UTC 
Created attachment 171508 [details]
Accomodate long lines in ul(1) - white space changes removed
Comment 2 Pietro Cerutti freebsd_committer freebsd_triage 2016-06-17 14:43:19 UTC 
https://reviews.freebsd.org/D6881
Comment 3 Abhinav Upadhyay 2016-06-17 18:30:00 UTC 
Comment on attachment 171508 [details]
Accomodate long lines in ul(1) - white space changes removed

Thanks for creating the bug and patch. I have two suggestions:

I think it would be better to do err(3) if realloc() fails to allocate memory, because otherwise, it's the same problem about the user not getting to know whether the program finished successfully or not.

Also, why still maintain the static buffer, wouldn't it be much simpler if you just had one dynamic buffer? Allocating 512 bytes at startup is not much overhead :)
Comment 4 Pietro Cerutti freebsd_committer freebsd_triage 2016-06-20 12:54:46 UTC 
(In reply to Abhinav Upadhyay from comment #3)

To err(3) would prevent processing of subsequent files. I am more inclined on having some lines truncated.

I don't see the proposed patch as highly complex. I think avoiding heap allocation for most files is still good.
Comment 5 Abhinav Upadhyay 2016-06-20 15:51:28 UTC 
(In reply to Pietro Cerutti from comment #4)

But there is precedent for exiting with err(3) when failing to open the file (see in main), so why continue here?

I don't mind about maintaining the static buffer, I was just curious :)
Comment 6 commit-hook freebsd_committer freebsd_triage 2016-07-11 08:06:23 UTC 
A commit references this bug:

Author: gahr
Date: Mon Jul 11 08:05:46 UTC 2016
New revision: 302558
URL: https://svnweb.freebsd.org/changeset/base/302558

Log:
  Do not truncate lines longer than 512 chars.

  PR:		210344
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D6881

Changes:
  head/usr.bin/ul/ul.c
Comment 7 commit-hook freebsd_committer freebsd_triage 2016-07-20 07:35:26 UTC 
A commit references this bug:

Author: gahr
Date: Wed Jul 20 07:33:48 UTC 2016
New revision: 303075
URL: https://svnweb.freebsd.org/changeset/base/303075

Log:
  MFC r302558:
  Do not truncate lines longer than 512 chars.

  PR:		210344
  Differential Revision:	https://reviews.freebsd.org/D6881

Changes:
_U  stable/10/
  stable/10/usr.bin/ul/ul.c
Comment 8 commit-hook freebsd_committer freebsd_triage 2016-10-19 08:24:02 UTC 
A commit references this bug:

Author: gahr
Date: Wed Oct 19 08:23:55 UTC 2016
New revision: 307619
URL: https://svnweb.freebsd.org/changeset/base/307619

Log:
  MFC r302558:

  Do not truncate lines longer than 512 chars.

  PR:		210344
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D6881

Changes:
_U  stable/11/
  stable/11/usr.bin/ul/ul.c