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

On 11/06/16 17:27, Tobias Stoeckmann wrote:

On Fri, Jun 10, 2016 at 07:11:16PM -0400, Andrew Gregory wrote:

...

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.

Yup, it's irrelevant. The relevant part is using strndup() with an input line. Maybe you want to adjust that one?

Fixing that strndup is preferable. I want to keep the ability to handle \0 delimited fields given that is what pacman --machinereadable does (despite that option not being ubiquitous at the moment...) Allan

On Sat, Jun 18, 2016 at 06:44:01PM +0200, Tobias Stoeckmann wrote:

On Mon, Jun 13, 2016 at 04:01:17PM +1000, Allan McRae wrote:

...

Fixing that strndup is preferable. I want to keep the ability to handle \0 delimited fields given that is what pacman --machinereadable does (despite that option not being ubiquitous at the moment...)

If \0 is a valid field _and_ line delimiter, the parsed input becomes ambiguous. How is that supposed to be handled?

Tobias

\n is a valid field and line delimiter, too. We should treat both of these cases the same -- catch them and reject them as invalid usage. d