On 06/11/16 at 12:14am, Tobias Stoeckmann wrote:
On Thu, Jun 09, 2016 at 07:46:00PM -0400, Andrew Gregory wrote:
-t and -k are used to sort tabular data, where NUL is a perfectly sane field separator. In fact, with --machinereadable pacman will output a table of NUL separated fields. Though, pacsort does need its handling of -t '\0' fixed in other places as well.
The function escape_char is only called from main for the 't' option argument. The result is stored in opts.delim.
The value of opts.delim is only used in function nth_column. That function is called in compare_versions, which is called by vercmp either directly or through compare_files.
And that one is called by main again. Which "other places" do you mean that have to be fixed? There is only one.
The parsing of input happens in input_new, which allocates space for in->data by using strndup(). Whatever points into this new data area, accessing past the first '\0' is a straight out of boundary access. No matter the origin of pacsort's input, first '\0' definitely means 'end here'.
Maybe you mixed up the stand alone utility "pacsort" with some function in libalpm?
Otherwise please let me know what exactly is broken now by showing the code path or actual program invocation.
I was saying that your rationale for just dropping support for -t '\0' was incorrect. NUL not being legal in a path name is irrelevant. But, as you just pointed out, pacsort has more issues with -t '\0' than just the poor use of strchr referenced by your patch. Hence, "pacsort does need its handling of -t '\0' fixed in other places as well." In other words, if -t '\0' is kept, pacsort needs to be fixed in a few places. apg