Windows support #510

Piloalucard · 2025-04-18T04:32:41Z

Summary of changes

This pull request includes following changes or fixes.

Compatible code with Microsoft Visual C++ Compiler
Support of Windows Server 2022
Support of Windows Server 2025
Support of Windows 11

Description

Code was updated to be compatible again with Windows platform. Microsoft mandates compilation with Microsoft tools to give support, with that said the code needed to be compatible with Microsoft Visual C++ Compiler for a new version to be built.
Macros we're added to comply with this and change the behavior depending on which platform the project is being built.

Type of change

Refactoring
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have confirmed my fix is effective or that my feature works

Author: Author Name vandel_van@hotmail.com

…ndows

piste-jp · 2025-04-21T01:47:50Z

I would like to notify this comment in advance before this PR goes to compilable...

I know you are working so hard on this PR and also I know you want to support windows again in the IBM storage archive product. But I must strongly disagree to this PR. Because this PR has no benefit to the normal open source users who are using the code in this repository. On the other hand code change is big and impact of the readability of codes are really big.

I have so many comments and complains but top-3 of my comments are

First, please don't introduce crossbuild directory easily. This kind of architecture specific code is handled under libltfs/arch directory. Please follow this as much as possible.

Second, please don't use SAFE_ prefix in the macros in commons.h. Frankly I cannot understand what does SAFE_ means. I believe they are "safe" only in windows environment for for Linux/Mac/BSDs env. I don't recommend that we use the C++11/C99 "Annex K" functions because of its history, they are optional and they are not supported in the glibc yet. What is the problem that LTFS uses current ANSI C standard functions on windows env? I strongly recommend that you need arch_ prefix and design glue layer carefully like thread handling in LTFS, see src/libltfs/ltfs_thread.h carefully.

For example, SAFE_FOPEN looks only safe for windows. And SAFE_WRITE cannot be understand what is "safe" at all.

Last, please write macros to keep the C language context. I believe macro feature can easily ruins the C language context and create new your own langage. But this project is clearly written by C please do not corrupt the readability for C developers. I strongly recommend that you need to use inline function as much possible, that is a reason taht we choose C99.

For example, this macro call confuses C developers in short period. They don't think conf_file is output of function (macro) at a glance. Of course, they can understand soon, but that small thinking time reduces the readability totally.

SAFE_FOPEN(path, "rb", conf_file);

This reverts commit 092238d.

piste-jp

Why do you want to use arch_ prefix for POSIX functions.

I believe your change doesn't make this code compilable on Visual C. This just a part of Visual C compilation only in IBM.

So I believe you need to minimize the code change because there is no benefit to this project at all!

My recommendation is just use POSIX functions as POSIX function name.

Honestly, I cannot understand why you want to replace POSIX function name to name with arch_ prefix. But if it is to suppress the waring message in Visual C, please suppress it with pragma declaration like below.

#ifdef _MSC_VER
#pragma warning(suppress : 4996)
...
#endif

See also, https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/posix-write?view=msvc-170.

Second best is

#ifdef _MSC_VER
#define write _write
...
#endif

src/utils/ltfsck.c

piste-jp · 2025-04-30T01:54:14Z

src/libltfs/arch/ltfs_arch_ops.h

+#define INVALID_KEY UINT_MAX
+
+
+    #define arch_vsprintf(buffer,bufferCount, fmt, ...) vsprintf_s((buffer), (bufferCount), (fmt), __VA_ARGS__)


Please do not use camel case bufferCount.

This is macro definition, so long variable name is not needed.

piste-jp

I roughly make comments. They are not everything.

I just at a glance and make comments.

piste-jp · 2025-04-30T02:04:38Z

src/iosched/unified.c

@@ -1764,7 +1764,7 @@ ssize_t _unified_insert_new_request(const char *buf, off_t offset, size_t count,
 	memcpy(cache_manager_get_object_data(*cache), buf, copy_count);

 	/* Store new write request */
-	new_req = calloc(1, sizeof(struct write_request));
+	new_req = (struct write_request*)calloc(1, sizeof(struct write_request));


I'm not sure why this cast is needed.

piste-jp · 2025-04-30T02:05:28Z

src/iosched/unified.c

 	struct timer_info timerinfo;
 	struct unified_data *priv = iosched_handle;
-
+	


Do not keep the leading spaces in an empty line.

piste-jp · 2025-04-30T02:08:45Z

src/iosched/unified.c

@@ -2325,7 +2325,7 @@ int unified_set_profiler(char *work_dir, bool enable, void *iosched_handle)
 			return -LTFS_NO_MEMORY;
 		}

-		p = fopen(path, PROFILER_FILE_MODE);
+		arch_fopen(path, PROFILER_FILE_MODE,p);


why fopen_s() is needed on windows?

piste-jp · 2025-04-30T02:14:29Z

src/kmi/simple.c

-		strcat((char *) priv.dk_list, (char *) key[i].dk);
-		strcat((char *) priv.dk_list, ":");
-		strcat((char *) priv.dk_list, (char *) key[i].dki);
+			arch_strcat((char *) priv.dk_list, dk_list_len, "/");


I cannot　understand why strcat_s() is needed here. Every input string can be handled safely in this function...

If you have a reason please explain it by comment and add strcat_s() function into linux side. I don't like Annex K, but it is better than this, if you have a strong reason.

piste-jp · 2025-04-30T02:16:38Z

src/libltfs/arch/ltfs_arch_ops.h

+
+    #define arch_access(filename, mode) _access((filename), (mode))
+
+    #define arch_xmlfree(ptr) xmlFree((ptr))


I recommend this.

#define xmlfree xmlFree

piste-jp · 2025-04-30T02:17:19Z

src/libltfs/arch/ltfs_arch_ops.h

+
+    #define arch_access(filename, mode) access(filename, mode)
+
+    #define arch_xmlfree(ptr) arch_safe_free(ptr)


I recommend that this line is removed.

piste-jp · 2025-04-30T02:24:31Z

src/libltfs/index_criteria.c

-	char rule[len+1], last, *ptr;
-
+	char *rule = NULL, last, *ptr;
+	int ruleLen = (sizeof(char) * (int)(len + 1));


Please do not use camel case.

piste-jp · 2025-04-30T02:25:20Z

src/libltfs/index_criteria.c

+	int ruleLen = (sizeof(char) * (int)(len + 1));
+	rule = (char*)calloc(ruleLen,sizeof(char));
+	if (rule == NULL) {
+		return -LTFS_NO_MEMORY;


Error message is needed.

piste-jp · 2025-04-30T02:26:16Z

src/libltfs/index_criteria.c

+	rulebuf = (char*)malloc(sizeof(char) * (len + 1));
+	if (rulebuf == NULL)
+	{
+		return -LTFS_NO_MEMORY;


Error message is needed.

piste-jp · 2025-05-01T07:30:45Z

Hmm, this PR has so many changes but no benefit to the original code users. Because they don't have any code built on windows even if your change is committed.

Let me clarify how to progress.. I strongly recommend that you start over from scratch again.

Objective

Change the code to be built on Visual C for the IBM product. But the tree cannot be built for this project.

Directions

Keep original code as much as possible, minimize changes
Do not make any refactoring and other prettifications
Drop MinGW support (move to Visual C support)

Changes Detail

Use POSIX functions as is (suppress warnings with #pragma as much as possible for windows)
Do not use Annex K functions as little s possible as (suppress warnings with #pragma as much as possible for windows)
Use inline wrapper functions for above if it is really needed

Piloalucard added 5 commits April 17, 2025 18:07

Cherry pick from build-windows

184d916

Cherry pick from build-windows

f3a4e82

Update make_message_src.sh

3bd0ba0

Undoing unwanted changes and comments

8b88246

Update compatibility.h

81b2ff4

Piloalucard requested review from amissael95, syaoraang, juliocelon, vandelvan, rangelmx and chukero April 18, 2025 04:32

Piloalucard self-assigned this Apr 18, 2025

Corrections for Linux build with windows changes

15e69c6

vandelvan force-pushed the windows branch 4 times, most recently from bafbd71 to 07a2da6 Compare April 18, 2025 23:26

Corrections for Linux build with windows changes

a7a2591

Author: Author Name vandel_van@hotmail.com

vandelvan force-pushed the windows branch from 07a2da6 to a7a2591 Compare April 18, 2025 23:29

Merge branch 'windows' of https://github.com/Piloalucard/ltfs into wi…

80395fa

…ndows

Piloalucard and others added 9 commits April 24, 2025 11:47

Changed macros to arch ops

9b03b94

rewriting functions

f2ed251

corrections for Linux build

c8c70bf

Making non-static for linux

46488e1

Update pathname.c

c49d6b6

Update pathname.c

b68e771

Update ltfs_arch_ops.h

ba335dc

Merge branch 'v2.4-stable' into windows

2ef6dec

Update ltfs_arch_ops.h

ebbb858

vandelvan added 3 commits April 28, 2025 15:54

updated python2 to python3 for validate error messages

092238d

Revert "updated python2 to python3 for validate error messages"

14a7620

This reverts commit 092238d.

fix: warnings during build in linux

757a2ed

piste-jp requested changes Apr 30, 2025

View reviewed changes

piste-jp reviewed Apr 30, 2025

View reviewed changes

Piloalucard added 4 commits April 30, 2025 08:55

Asterisk fix in pointers decl to match standard

83f8380

Update ltfs_arch_ops.h

35760f9

Update ltfs_arch_ops.h

45662c0

Updated arch_ops and added missing error msgs

961c2b9

vandelvan removed their request for review May 2, 2025 17:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows support #510

Windows support #510

Piloalucard commented Apr 18, 2025 •

edited

Loading

piste-jp commented Apr 21, 2025

piste-jp left a comment •

edited

Loading

piste-jp Apr 30, 2025

piste-jp left a comment

piste-jp Apr 30, 2025

piste-jp Apr 30, 2025

piste-jp Apr 30, 2025

piste-jp Apr 30, 2025

piste-jp Apr 30, 2025

piste-jp Apr 30, 2025

piste-jp Apr 30, 2025

piste-jp Apr 30, 2025

piste-jp Apr 30, 2025

piste-jp commented May 1, 2025

		#define INVALID_KEY UINT_MAX


		#define arch_vsprintf(buffer,bufferCount, fmt, ...) vsprintf_s((buffer), (bufferCount), (fmt), __VA_ARGS__)

		struct timer_info timerinfo;
		struct unified_data *priv = iosched_handle;


		#define arch_access(filename, mode) _access((filename), (mode))

		#define arch_xmlfree(ptr) xmlFree((ptr))


		#define arch_access(filename, mode) access(filename, mode)

		#define arch_xmlfree(ptr) arch_safe_free(ptr)

Windows support #510

Are you sure you want to change the base?

Windows support #510

Conversation

Piloalucard commented Apr 18, 2025 • edited Loading

Summary of changes

Description

Type of change

Checklist:

piste-jp commented Apr 21, 2025

piste-jp left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piste-jp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piste-jp commented May 1, 2025

Objective

Directions

Changes Detail

Piloalucard commented Apr 18, 2025 •

edited

Loading

piste-jp left a comment •

edited

Loading