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.

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

fix C++ compile errors (FreeBSD-current) #1245

Merged
merged 1 commit into from
May 25, 2024
Merged

Conversation

cnbatch
Copy link
Contributor

@cnbatch cnbatch commented May 17, 2024

@@ -63,7 +63,7 @@

#define _NLA_END(_start, _len) ((char *)(_start) + (_len))
#define NLA_FOREACH(_attr, _start, _len) \
for (_attr = (_start); \
for (_attr = (typeof(_attr))(_start); \
Copy link
Member

Choose a reason for hiding this comment

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

Does this work with C still?

Copy link
Contributor

Choose a reason for hiding this comment

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

typeof is a compiler extension, it was added in C23.

Copy link
Member

Choose a reason for hiding this comment

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

So no then. We don't enable c23 for the kernel at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to use __typeof instead

Copy link
Contributor

Choose a reason for hiding this comment

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

Uh how about doing...

#ifdef __cplusplus
# define NLA_FOREACH(_attr, _start, _len)      \
        for (_attr = (decltype(_start))(_start);	\
		((char *)_attr < _NLA_END(_start, _len)) && \
		((char *)NLA_NEXT(_attr) <= _NLA_END(_start, _len));	\
		_attr =  NLA_NEXT(_attr))
#else
# define NLA_FOREACH(_attr, _start, _len)      \
        for (_attr = (_start);		\
		((char *)_attr < _NLA_END(_start, _len)) && \
		((char *)NLA_NEXT(_attr) <= _NLA_END(_start, _len));	\
		_attr =  NLA_NEXT(_attr))
#endif

This one would be valid C and C++ code (without using any compiler extensions)... but the only issue (visually) is that two similar macros, one for C and one for C++

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Splitting it into two macros might make future maintenance inconvenience.

How about I change it to (struct nlattr *) just like NLA_NEXT at line 62?
NLA_FOREACH macro uses NLA_NEXT internally, and NLA_NEXT is directly cast as (struct nlattr *).

Copy link
Contributor

Choose a reason for hiding this comment

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

So no then. We don't enable c23 for the kernel at all.

typeof() should be fine in practice. Compiling the following fragment on compiler explorer says clang >= 11 and gcc >= 10 support it with default flags.

int a;
typeof(a) b;

sys/netlink/netlink_snl.h Outdated Show resolved Hide resolved
@@ -102,7 +102,7 @@ lb_allocz(struct linear_buffer *lb, int len)
len = roundup2(len, alignof(__max_align_t));
if (lb->offset + len > lb->size)
return (NULL);
void *data = (void *)(lb->base + lb->offset);
char *data = (lb->base + lb->offset);
lb->offset += len;
return ((char *)data);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you've used char *data ... you can safely remove this cast

Copy link
Member

Choose a reason for hiding this comment

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

This looks to be the only active comment, and the only thing that has caught my eye that's in the way of committing.

@bsdimp bsdimp self-assigned this May 23, 2024
Copy link
Member

@bsdimp bsdimp left a comment

Choose a reason for hiding this comment

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

I think this is now good

Allow these files to be included in C++ programs with careful casting to
the proper type, like C++ wants (and in a way that also works for C).

MFC After: 1 week
Reviewed by: imp
Pull Request: freebsd#1245
@freebsd-git freebsd-git merged commit ff92493 into freebsd:main May 25, 2024
6 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants