frr icon indicating copy to clipboard operation
frr copied to clipboard

lib: Discourage usage of deprecated data structures

Open donaldsharp opened this issue 1 year ago • 8 comments

Put some verbiage in place to warn people that we are actively discouraging new development that uses an older data structure.

donaldsharp avatar May 31 '24 16:05 donaldsharp

I would prefer that we stop using the BSD variants and get them removed from the repository in the long term.

donaldsharp avatar Jun 01 '24 14:06 donaldsharp

I would prefer that we stop using the BSD variants and get them removed from the repository in the long term.

Figured, but maybe we shouldn't refer to using the type safe variants instead when talking about moving away from type safe variants :) Maybe call them FRR native type safe variants (or maybe better name them explicitly?)

choppsv1 avatar Jun 01 '24 14:06 choppsv1

Agree with Donald, BSD variants are kinda OK, but we need to sync them (same as we do for some Linux headers...).

ton31337 avatar Jun 02 '24 15:06 ton31337

Agree with Donald, BSD variants are kinda OK, but we need to sync them (same as we do for some Linux headers...).

This ship has probably already sailed, but ...

The problem with our type safe macros, IMO, is that when you use an IDE, the FRR type safe stuff is hyper obfuscated (and in general I find the generated macro names somewhat non-noticeable as well), whenever you "Go to definition" to see what a function call is doing (e.g., dplane_ctx_list_add_tail) they all lead you back to a single MACRO (e.g., DECLARE_DLIST(dplane_ctx_list...), and not to the code tha actually adds something to the tail of a list.

The BSD macros are just as type-safe but much more obvious (and obviously named e.g., TAILQ_INSERT_TAIL() and you actually are taken to the code that is implements the operation. I think that's valuable.

choppsv1 avatar Jun 03 '24 18:06 choppsv1

Fixing our typesafe code is a bit orthogonal to what I am trying to do here. I would just like to let people know that they should think about using something different.

donaldsharp avatar Jun 04 '24 14:06 donaldsharp

Fixing our typesafe code is a bit orthogonal to what I am trying to do here. I would just like to let people know that they should think about using something different.

My original comment was that referring to "typesafe" meaning "look elsewhere" in the openbsd-queue.h file is imprecise since the things defined in that header can be called "typesafe" and you're trying to point people away from them. :)

FWIW, I absolutely support putting the comments in the hash.h and linklist.h headers, without modification.

Now, my personal opinion is that we shouldn't put these comments in the openbsd header files, but if that's too big an ask perhaps at least we don't suggest people convert existing use away from them?

choppsv1 avatar Jun 04 '24 14:06 choppsv1

just to record, the consensus on yesterday's community call was to add some additional text on the BSD datastructures along the lines of However, among converting datastructures, the BSD ones are the lowest priority / should be converted last. They are already typesafe and use inline linking nodes, so the only gain is consistency. Please convert uses of linklist.h and hash.h first.

eqvinox avatar Jun 19 '24 10:06 eqvinox

updated

donaldsharp avatar Jun 19 '24 11:06 donaldsharp