tass-belgium / picotcp

PicoTCP is a free TCP/IP stack implementation
Other
1.19k stars 219 forks source link

There are cyclic dependencies between header files #442

Open phalox opened 7 years ago

phalox commented 7 years ago

Since the precompiler only does a single run, this was never an issue. However, now that I'm trying to parse the order of inclusion for these files it does become an issue. These files were identified as having a cyclic dependency:

PROBLEM 1

 ('pico_addressing.h', ['pico_config.h', 'pico_constants.h']),
 ('pico_constants.h', ['pico_addressing.h']),

I don't think pico_constants needs pico_addressing. And if it really does, then some things should be moved so that it doesn't

PROBLEM 2

 ('pico_config.h', ['pico_defines.h', 'pico_constants.h', 'pico_mm.h', 'pico_port.h']),
 ('pico_mm.h', ['pico_config.h']),

I don't think pico_mm needs pico_config. But the cleaner way to fix this would be the other way around (pico_config not having to need pico_mm)

I believe we should be able to get rid of them by moving or deleting certain items.

frederikvs commented 7 years ago

would PR #450 solve this issue?

phalox commented 7 years ago

Nice, didn't see this PR yet.

Looking at the PR, I don't think it does entirely, but it does contribute indeed. There will still be a circular dependency, which you'll never notice while just compiling the code.

You'll need specifically https://github.com/tass-belgium/picotcp/commit/6293b6a54eb4db80c11cbbd32c3d47756955abb8#diff-3173832207104af4d8aca370cc31cab2R27 and https://github.com/tass-belgium/picotcp/commit/6293b6a54eb4db80c11cbbd32c3d47756955abb8#diff-95c9b02dbc22b692d372658b1255cf90R12

Though merging in that PR could already be a good step!