Skip to content

Conversation

@cmgauger
Copy link
Contributor

Move the eth_copy_mac() and eth_mac_cmp() functions created in commit c20b391 into the sim_ether.c file as actual functions, as Visual Studio 2013 and below do not support the inline keyword for C. This change also allows for pre-ANSI C99 compilers to continue compiling SIMH (as inline was not added until C99).

@bscottm
Copy link
Contributor

bscottm commented Oct 1, 2025

You could have used SIM_INLINE instead of making them actual functions in sim_ether.c. That way, the functions would just be local to the source that used them in the case of older non-C99 compilers, since they are one line calls to memcpy and memcmp.

@bscottm
Copy link
Contributor

bscottm commented Oct 1, 2025

Looks like I need to figure out what's going on with the current GH CI/CD images too.

@cmgauger
Copy link
Contributor Author

cmgauger commented Oct 1, 2025

You could have used SIM_INLINE instead of making them actual functions in sim_ether.c. That way, the functions would just be local to the source that used them in the case of older non-C99 compilers, since they are one line calls to memcpy and memcmp.

Well if I had known of the existence of SIM_INLINE I'd have used it; let me fix that then.

@cmgauger
Copy link
Contributor Author

cmgauger commented Oct 1, 2025

Done, I have reverted d2e4d5e, and created 8433091 which uses SIM_INLINE.

@markpizz
Copy link
Contributor

markpizz commented Oct 1, 2025

Your changes should be squashed into a single commit.

One way to do this might be:

% git reset --soft head^^^
% git commit .....
% git push -f to your github repo.

Replaced the naked `inline` specifier with the SIM_INLINE macro call,
allowing for pre-ANSI C99 compilers to continue compiling SIMH, as the
`inline` keyword is unsupported in ANSI C89/ISO C90.
@cmgauger
Copy link
Contributor Author

cmgauger commented Oct 2, 2025

Your changes should be squashed into a single commit.

I've gone and done that.

@cmgauger
Copy link
Contributor Author

cmgauger commented Oct 2, 2025

Should also mention that, at least in terms of MS Visual Studio versions prior to and including VS 2013, that #474 is required. But this can be implemented without #474 as the fix does regularize the function definitions to the rest of the code base's style.

@bscottm
Copy link
Contributor

bscottm commented Oct 13, 2025

@cmgauger (Christian/Chris): Thanks for resubmitting with SIM_INLINE. Maintaining, enhancing and ensuring interoperability open-simh is a team effort and there's a lot of undocumented foo, like SIM_INLINE. Appreciate you pointing out an interoperability issue and taking action to remedy.

Off-topic: Does VS 2013 allow for named structure members in initialization, e.g.,:

    SDL_Event user_event = {
        .user.type = evidentifier_for(SIMH_EVENT_OPEN),
        .user.code = 0,
        .user.data1 = vptr,
        .user.data2 = &did_opencomplete
    };

@LegalizeAdulthood
Copy link

Is it really a priority to support a 12 year old compiler, when a current compiler is freely available?

Use old code with old compilers is what I always recommend.

@pkoning2
Copy link
Member

Is it really a priority to support a 12 year old compiler, when a current compiler is freely available?

Use old code with old compilers is what I always recommend.

Priority? Maybe not. But a simple change to enable it to work seems like a reasonable thing to do.

@cmgauger
Copy link
Contributor Author

Is it really a priority to support a 12 year old compiler, when a current compiler is freely available?

Use old code with old compilers is what I always recommend.

This PR regularizes the inline invocation to being the same as in the rest of SIMH; that it makes it work on VS 2010 (if #474 is implemented...) is a nice side benefit (for me, as I use VS 2010).

As far as supporting a twelve year old compiler goes: well, SIMH still supports builds on Compaq C v6.4 on OpenVMS v7.3 on the VAX, and that's twenty-one years old.

@cmgauger
Copy link
Contributor Author

Off-topic: Does VS 2013 allow for named structure members in initialization, e.g.,:

A quick look says that designated initializers was added in VS 2013 (it's not in VS 2010 though). Although admittedly I never use them (since my style of C is mostly C89-but-I-use-stdint.h).

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.

5 participants