On Tue, 9 Jun 2015, Greg Troxel wrote:

Having read the comments at 407, I'm not at all comfortable that I understand what's going on. It's not clear (from the missing commit messages in these patches :-) where the matching lock acquisitions are, why it's ok to use the variables after the unlocks, why info is null if unlock is called, etc.

Ah, the joys of our refcounting.

lookup and get both automatically bump the ref-count on a returned object. The caller decs if it does /not/ store a reference.

The (rn->info == info) check on a route_node_get tells you whether route_node_get created the object on that call - in which case you want to /keep/ the refcount because that's the one for the table reference that route_node_get stored and assign the info pointer! Otherwise, if it already had an assigned info pointer, and you're not otherwise storing the thing elsewhere, then you need to undo that decrement.

It's conflating the refcount for the route_node object with the refcount for the actual indexed object somewhat, which is confusing.

This looks like it is gratuitously dropping a lot of const, and I don't
see the point.  Maybe that's because I didn't read the missing commit
message.

;)

A lot of older APIs aren't const, even where they should be.

Actually, it looks like maybe this patch could just change run_background to (char *const)??

regards,
--
Paul Jakma      [email protected]  @pjakma Key ID: 64A2FF6A
Fortune:
%DCL-MEM-BAD, bad memory
VMS-F-PDGERS, pudding between the ears

_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev

Reply via email to