bin/182098: [patch] Change kldxref fts_open ordering so it produces a consistent linker.hints between machines of the same architecture.

[Derek Schrock]

The following reply was made to PR bin/182098; it has been noted by GNATS.

From: Derek Schrock <dereks at lifeofadishwasher.com>
To: Jilles Tjoelker <jilles at stack.nl>
Cc: bug-followup at FreeBSD.org
Subject: Re: bin/182098: [patch] Change kldxref fts_open ordering so it
 produces a consistent linker.hints between machines of the same
 architecture.
Date: Tue, 24 Sep 2013 21:32:42 -0400

 --NzB8fVQJ5HfG6fxh
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 On Mon, Sep 16, 2013 at 12:39:58AM +0200, Jilles Tjoelker wrote:
 > In the interest of reproducible builds, your patch seems a good idea. It
 > seems unattractive to run kldxref /boot/kernel on every machine.
 > 
 > The implementation of compare() seems unnecessarily complex though. In
 > find -s, the fts_names are simply passed to strcoll() (here, strcmp()
 > would be better). The trickery with the length may cause inconsistent
 > results if one filename is a prefix of another (rare).
 
 Fair enough after reading more of the fts(3) man fts_name is always null terminated.
 
 > 
 > This change may also expose a latent bug with kldxref -R: it does not
 > work properly if a directory contains both files that need a mention in
 > a hints file and subdirectories, and at least one such file appears
 > after a subdirectory. Because your change alters the traversal order, it
 > might break a use of kldxref -R that previously happened to work. You
 > can make it work reliably by sorting FTS_D entries after other entries.
 
 Yep, after some additional testing with the patched kldxref it produced different linker.hints files. 
 
 The new patch uses fts_parent's fts_name (the directory's name).  First compare the parent's name, if the same compare the passed FTSENTs.
 
 --NzB8fVQJ5HfG6fxh
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename="patch.txt"
 
 --- kldxref.c.old	2013-09-24 21:06:07.000000000 -0400
 +++ kldxref.c	2013-09-24 21:31:39.000000000 -0400
 @@ -275,6 +275,19 @@
  	exit(1);
  }
  
 +int 
 +compare(const FTSENT* const* a, const FTSENT* const* b)
 +{
 +	char* a_parent = ((*a)->fts_parent)->fts_name;
 +	char* b_parent = ((*b)->fts_parent)->fts_name;
 +	int match = strcmp(a_parent, b_parent);
 +
 +	if (match == 0)
 +		match = strcmp((*a)->fts_name, (*b)->fts_name);
 +
 +	return match;
 +}
 +
  int
  main(int argc, char *argv[])
  {
 @@ -316,7 +329,7 @@
  		err(1, "%s", argv[0]);
  	}
  
 -	ftsp = fts_open(argv, fts_options, 0);
 +	ftsp = fts_open(argv, fts_options, compare);
  	if (ftsp == NULL)
  		exit(1);
  
 
 --NzB8fVQJ5HfG6fxh--