Re: [Monotone-devel] automate inventory broken

[Timothy Brownawell]

On Sun, 2008-06-01 at 04:37 -0400, Stephen Leake wrote:
> Timothy Brownawell <address@hidden> writes:
> 
> > automate inventory is printing birth revisions for ignored files, which
> > breaks the automate_inventory test. The automate_inventory_restricted
> > test is also failing, which at first glance looks to be due to the same
> > problem.
> 
> These tests are passing for me on MinGW. Examining the test log shows no
> birth revisions for ignored files.
> 
> What platform are you running on?

Debian unstable.

> Hmm. Looking at the code, in automate.cc, there is:
> 
> static revision_id
> inventory_determine_birth(inventory_item const & item,
>                           roster_t const & old_roster,
>                           marking_map const & old_marking)
> {
>   revision_id rid;
>   if (old_roster.has_node(item.new_node.id))
>     {
>       node_t node = old_roster.get_node(item.new_node.id);
>       marking_map::const_iterator m = old_marking.find(node->self);
>       I(m != old_marking.end());
>       marking_t mark = m->second;
>       rid = mark.birth_revision;
>     }
>   return rid;
> }
> 
> rid is not explicitly set if the 'if' condition is false. 
> 
> Does 'revision_id' have a default constructor? I can't figure out the
> vocab macros.
> 
> If it doesn't have a constructor, 'rid' will have a random value from
> the stack. 

If it doesn't have a default constructor, that should be a compile
error.

But item.new_node.id should be a node_id, which is just an int of some
sort and *can* be uninitialized... and it looks like the node_info
constructor doesn't initialize it, because it wants us to check 'exists'
before using any other fields. But it's probably safer to just
initialize it to the_null_node, so forgetting that check won't break
things.

struct node_info
{
  bool exists;
  // true if node_id is present in corresponding roster with the inventory map 
file_path
  // false if not present, or present with a different file_path
  // rest of data in this struct is invalid if false.
  node_id id;
  path::status type;
  file_id ident;
  full_attr_map_t attrs;

  node_info() : exists(false), id(the_null_node), type(path::nonexistent) {}
};