Skip to content

[GEN][ZH] Add endian compat for BIGFileSystems #798

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
May 7, 2025

Conversation

Mauller
Copy link

@Mauller Mauller commented May 2, 2025

This PR is a prerequisit to a PR to unify AsciiString, it adds endian compatibility functions within an endian_compat header.

These are then used in the BIGFileSystems as they previously reloed on the networking ntohl function.
For StdBIGFileSystem this was being indirectly provided through the windows header in AsciiString. And directly included in win32BigFileSystem through winsock2.

This change is necessary to provide cross compat byte swapping functionality.

@Mauller Mauller changed the title [ZH] Add Bswap intrinsics for StdBIGFileSystem [ZH] Add Byte swapping intrinsics for StdBIGFileSystem May 2, 2025
@Mauller Mauller self-assigned this May 2, 2025
@Mauller Mauller added Minor Severity: Minor < Major < Critical < Blocker ZeroHour Relates to Zero Hour Refactor Improves the structure of the code, with negligible changes in function. labels May 2, 2025
@xezon xezon requested a review from OmniBlade May 3, 2025 11:40
@Mauller Mauller force-pushed the bswap-intrinsics branch from 3d48760 to cfa2a8b Compare May 5, 2025 15:09
@Mauller
Copy link
Author

Mauller commented May 5, 2025

Updated with a more fleshed out endian handling compatability header.

@Mauller Mauller changed the title [ZH] Add Byte swapping intrinsics for StdBIGFileSystem [ZH] Add endian compat for StdBIGFileSystem May 5, 2025
@Mauller Mauller force-pushed the bswap-intrinsics branch from cfa2a8b to 4247e96 Compare May 5, 2025 16:15
@Mauller Mauller force-pushed the bswap-intrinsics branch from 4247e96 to 23569f3 Compare May 5, 2025 17:39
@Mauller Mauller force-pushed the bswap-intrinsics branch from 23569f3 to bff165e Compare May 5, 2025 19:16
@Mauller Mauller changed the title [ZH] Add endian compat for StdBIGFileSystem [GEN][ZH] Add endian compat for BIGFileSystems May 6, 2025
@Mauller Mauller force-pushed the bswap-intrinsics branch 2 times, most recently from 3a98c64 to de0f8f7 Compare May 6, 2025 17:32
@Mauller
Copy link
Author

Mauller commented May 6, 2025

Updated, Rebased off main and merge conflict fixed.

Tweaked with discussed changes and added endian compat functions to win32Bigfilesystems for generals and zero hour along with stdbigfilesystem.

Should be good to go now.

@xezon
Copy link

xezon commented May 6, 2025

VC6 does not like the variadic macro. Then we need to do #define static_assert(a,b) and we settle on static asserts always requiring 2 arguments for now.

@Mauller
Copy link
Author

Mauller commented May 6, 2025

VC6 does not like the variadic macro. Then we need to do #define static_assert(a,b) and we settle on static asserts always requiring 2 arguments for now.

Yeah i just noticed this, was just about to change it, should be up in a few seconds.

@Mauller Mauller force-pushed the bswap-intrinsics branch 2 times, most recently from 8759381 to 615da66 Compare May 6, 2025 18:12
@Mauller
Copy link
Author

Mauller commented May 6, 2025

Now it doesn't like long long, this is taking a long long time. i should really sort my local VC6 build environment lol.

Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Looks good.

@Mauller Mauller force-pushed the bswap-intrinsics branch from 615da66 to 06d6f69 Compare May 6, 2025 18:29
@Mauller
Copy link
Author

Mauller commented May 6, 2025

Looks good.

It would seem that VC6 doesn't like the helper templates

EDIT: Figured it out, VC6 doesn't support #pragma once and i didn't include traditional header guards.

EDIT2: Nvm wasn't that...

@Mauller Mauller force-pushed the bswap-intrinsics branch from 06d6f69 to 49836cb Compare May 6, 2025 19:01
@Mauller Mauller force-pushed the bswap-intrinsics branch 3 times, most recently from 8fe7c1e to 497f7af Compare May 6, 2025 20:01
@xezon
Copy link

xezon commented May 6, 2025

It is using what is called "partial template class specialization". Perhaps VC6 does not support that?

Chat Gippity says

No, Visual C++ 6.0 (VC6) does not support partial template class specialization.

While VC6 does support full class template specialization, it lacks support for partial specialization, which was not fully implemented in Microsoft's compiler until later versions (Visual Studio .NET 2003 and later improved this significantly).

Example of what VC6 does not support:

// This is partial specialization — not supported in VC6
template <typename T, typename U>
class MyClass {};

template <typename T>
class MyClass<T, int> {
    // ...
};

What VC6 does support:

// This is full specialization — supported in VC6
template <typename T, typename U>
class MyClass {};

template <>
class MyClass<int, float> {
    // ...
};

If you need to simulate partial specialization in VC6, you can sometimes use traits classes or template overloading with helper structs, but it's usually a bit hacky.

Here’s a workaround for partial class template specialization in Visual C++ 6.0 (VC6) using helper structs and full specialization, which VC6 does support.


🎯 Goal:

Simulate this partial specialization (which VC6 does not support):

template <typename T, typename U>
class MyClass;

template <typename T>
class MyClass<T, int> {
    // specialized for second type being int
};

✅ Workaround using helper class:

// Primary implementation — not specialized
template <typename T, typename U>
struct MyClassImpl {
    static void doSomething() {
        // Generic implementation
        std::cout << "Generic MyClassImpl<T, U>\n";
    }
};

// Full specialization of helper for U = int
template <typename T>
struct MyClassImpl<T, int> {
    static void doSomething() {
        std::cout << "Specialized MyClassImpl<T, int>\n";
    }
};

// Wrapper class
template <typename T, typename U>
class MyClass {
public:
    void doSomething() {
        MyClassImpl<T, U>::doSomething();
    }
};

🔧 Usage:

#include <iostream>

int main() {
    MyClass<double, float> a;
    MyClass<char, int> b;

    a.doSomething();  // Prints: Generic MyClassImpl<T, U>
    b.doSomething();  // Prints: Specialized MyClassImpl<T, int>
    return 0;
}

💡 Summary:

  • The key trick is to push the specialization into a secondary template, and call it from the main class.
  • VC6 can fully specialize that helper, even though it can't partially specialize the main template.

Do you want a version of this that works with older C++ features only (e.g. no typename, no std::cout)?

@Mauller
Copy link
Author

Mauller commented May 6, 2025

It is using what is called "partial template class specialization". Perhaps VC6 does not support that?

I have not used it before myself, just looking it up, but It's possibly not working due to a defect in the C++ 98 standard.
So VC6 might not support it with the way it has been implemented.

@Mauller Mauller force-pushed the bswap-intrinsics branch from 497f7af to bae21c6 Compare May 6, 2025 20:31
@Mauller
Copy link
Author

Mauller commented May 6, 2025

Will have a go at this again tomorrow.

@xezon
Copy link

xezon commented May 7, 2025

The Ai generated workaround looks strange however.

// Full specialization of helper for U = int
template <typename T>
struct MyClassImpl<T, int> {

It describes this as full specialization, but it still looks like partial. I expect this does not work.

@Mauller
Copy link
Author

Mauller commented May 7, 2025

The Ai generated workaround looks strange however.

// Full specialization of helper for U = int
template <typename T>
struct MyClassImpl<T, int> {

It describes this as full specialization, but it still looks like partial. I expect this does not work.

I was thinking that we might have to just do

#if defined(_msc_ver) && _msc_ver < 1300
Littlefiles = ntohl(Littlefiles)
#else
Littlefiles = betoh(Littlefiles)
#endif

In the win32 big file system then conditionally gate the endian compat to include winsock2 for vc6 and hide all the template code.

Copy link

@xezon xezon 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 we can do it simpler by creating inline functions. This should be sufficient for non-enum types.

inline uint16_t betoh(uint16_t x) ...
inline uint32_t betoh(uint32_t x) ...

@feliwir
Copy link

feliwir commented May 7, 2025

Letting you know that Linux & macOS both have ntohl , so not sure why we are doing a replacement

@xezon xezon added this to the Code foundation build up milestone May 7, 2025
@Mauller Mauller force-pushed the bswap-intrinsics branch from bae21c6 to f374f37 Compare May 7, 2025 16:51
@Mauller
Copy link
Author

Mauller commented May 7, 2025

Hopefully this is the one lol.

Checked the byte swap macros by having them get compiled in VS22 and they worked fine.

@Mauller
Copy link
Author

Mauller commented May 7, 2025

Tested the VC6 builds and they work as expected.

@Mauller Mauller force-pushed the bswap-intrinsics branch from f374f37 to f630194 Compare May 7, 2025 19:12
@Mauller
Copy link
Author

Mauller commented May 7, 2025

Refactored taking the suggestions into account.

Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Looks good.

@Mauller Mauller force-pushed the bswap-intrinsics branch from f630194 to fc0462f Compare May 7, 2025 20:34
@Mauller
Copy link
Author

Mauller commented May 7, 2025

Pushed with tweaks to the comments, should be good now.

@xezon
Copy link

xezon commented May 7, 2025

This was quite the journey :P

@Mauller
Copy link
Author

Mauller commented May 7, 2025

This was quite the journey :P

Always learning something along the way.

@xezon xezon removed the ZeroHour Relates to Zero Hour label May 7, 2025
@xezon xezon merged commit 4f1f489 into TheSuperHackers:main May 7, 2025
18 checks passed
@xezon xezon deleted the bswap-intrinsics branch May 7, 2025 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Minor Severity: Minor < Major < Critical < Blocker Refactor Improves the structure of the code, with negligible changes in function.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants