Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ospf6d: memcmp cleanup #10070

Merged
merged 2 commits into from
Nov 22, 2021
Merged

ospf6d: memcmp cleanup #10070

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8f359e1
ospf6d: remove meaningless ospf6_route_is_identical
idryzhov Nov 15, 2021
66314e9
ospf6d: replace memcmp with correct comparisons
idryzhov Nov 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
ospf6d: remove meaningless ospf6_route_is_identical 
As part of the check, it memcompares two structs ospf6_path. This struct
has a pointer field nh_list which is allocated every time a new path is
created, which means it can never be the same for two different paths.
Therefore this check is always false and can be completely removed.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
  • Loading branch information
@idryzhov
idryzhov committed Nov 15, 2021
commit 8f359e1593c41432273458b0c7bfd7cd27b47d45
21 changes: 0 additions & 21 deletions ospf6d/ospf6_route.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -704,27 +704,6 @@ struct ospf6_route *ospf6_route_add(struct ospf6_route *route,
}

if (old) {
/* if route does not actually change, return unchanged */
if (ospf6_route_is_identical(old, route)) {
Copy link
Member

@qlyoung qlyoung Nov 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than entirely remove this, shouldn't it now become a simple pointer comparison? If the comparison is always false for different struct instances as you say then we should still check if the instance is the same right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought about this, but it's not possible either. route is always a new object (lock is asserted to be 0), and old is always an existing object (lock is decremented).

if (IS_OSPF6_DEBUG_ROUTE(MEMORY))
zlog_debug(
"%s %p: route add %p: needless update of %p old cost %u",
ospf6_route_table_name(table),
(void *)table, (void *)route,
(void *)old, old->path.cost);
else if (IS_OSPF6_DEBUG_ROUTE(TABLE))
zlog_debug("%s: route add: needless update",
ospf6_route_table_name(table));

ospf6_route_delete(route);
SET_FLAG(old->flag, OSPF6_ROUTE_ADD);
ospf6_route_table_assert(table);

/* to free the lookup lock */
route_unlock_node(node);
return old;
}

if (IS_OSPF6_DEBUG_ROUTE(MEMORY))
zlog_debug(
"%s %p: route add %p cost %u paths %u nh %u: update of %p cost %u paths %u nh %u",
Expand Down
6 changes: 0 additions & 6 deletions ospf6d/ospf6_route.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,12 +299,6 @@ extern const char *const ospf6_path_type_substr[OSPF6_PATH_TYPE_MAX];
&& memcmp(&(ra)->path.origin, &(rb)->path.origin, \
sizeof(struct ospf6_ls_origin)) \
== 0)
#define ospf6_route_is_identical(ra, rb) \
((ra)->type == (rb)->type \
&& memcmp(&(ra)->prefix, &(rb)->prefix, sizeof(struct prefix)) == 0 \
&& memcmp(&(ra)->path, &(rb)->path, sizeof(struct ospf6_path)) == 0 \
&& listcount(ra->paths) == listcount(rb->paths) \
&& ospf6_route_cmp_nexthops(ra, rb) == 0)

#define ospf6_route_is_best(r) (CHECK_FLAG ((r)->flag, OSPF6_ROUTE_BEST))

Expand Down