Skip to content

Fix compatibility with ELL 0.45 and later #233

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

Merged
merged 1 commit into from
Jun 7, 2022

Conversation

mjmartineau
Copy link
Member

ELL 0.45 and later have header files that use a trailing semicolon with
the DEFINE_CLEANUP_FUNC() macro, which is not compatible with the
"-pedantic" flag in gcc. This causes the mptcpd code to issue the
following error:

"ISO C does not allow extra ‘;’ outside of a function [-Werror=pedantic]"

Wrap the ELL includes in some pragmas to avoid this error.

ELL 0.45 and later have header files that use a trailing semicolon with
the DEFINE_CLEANUP_FUNC() macro, which is not compatible with the
"-pedantic" flag in gcc. This causes the mptcpd code to issue the
following error:

"ISO C does not allow extra ‘;’ outside of a function [-Werror=pedantic]"

Wrap the ELL includes in some pragmas to avoid this error.
@coveralls
Copy link

coveralls commented Jun 6, 2022

Pull Request Test Coverage Report for Build 2450362651

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 61.588%

Totals Coverage Status
Change from base Build 2393071109: 0.0%
Covered Lines: 1148
Relevant Lines: 1864

💛 - Coveralls

@ossama-othman
Copy link
Member

ossama-othman commented Jun 7, 2022

Do you think it would be overkill to create a private mptcpd header for each ELL header that triggers the pedantic diagnostic? For example, we could have something like the following in <mptcpd/private/util.h>:

// SPDX-License-Identifier: BSD-3-Clause
/**
 * @file private/util.h
 *
 * @brief mptcpd convenience wrapper around <ell/util.h>.
 *
 * Copyright (c) 2022, Intel Corporation
 */

#ifndef MPTCPD_PRIVATE_UTIL_H
#define MPTCPD_PRIVATE_UTIL_H

/*
  ELL 0.45 and later have header files that use a trailing semicolon
  with the DEFINE_CLEANUP_FUNC() macro, which is not compatible with
  the "-pedantic" flag in gcc. This causes the mptcpd code to issue the
  following error:

  "ISO C does not allow extra ‘;’ outside of a function
  [-Werror=pedantic]"

  Wrap the ELL includes in some pragmas to avoid this error.
*/
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wpedantic"
#include <ell/util.h>
#pragma GCC diagnostic pop

#endif  // MPTCPD_PRIVATE_UTIL_H


/*
  Local Variables:
  c-file-style: "linux"
  End:
*/

Mptcpd source files would then include <mptcpd/private/util.h> instead of <ell/util.h>, and similarly for other ELL headers that trigger the -pedantic diagnostic. That would allow us to avoid peppering the mptcpd code with lots of #pragma directives, but at the cost of having more files to maintain.

@ossama-othman ossama-othman self-assigned this Jun 7, 2022
@mjmartineau
Copy link
Member Author

mjmartineau commented Jun 7, 2022

The wrapper headers do seem like a heavier solution to me.

I also considered:

#include <mptcpd/private/util.h>

BEGIN_ELL_INCLUDE
#include <ell/this.h>
#include <ell/that.h>
END_ELL_INCLUDE

where private/util.h (or whatever) has the macro definitions containing the pragmas.

but it doesn't save any lines of code, and at least directly using the pragmas shows any future developer what exactly is going on when they add a new source file.

I don't have strong feelings about the exact flavor of the solution, I just don't want to burn much more time on it...

@ossama-othman ossama-othman merged commit 71191d9 into multipath-tcp:master Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants