[Patch] New VHD support... now with added dynamics

User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [Patch] New VHD support... now with added dynamics

Post by SarahWalker »

shermanp wrote: Sun 15 Nov, 2020 6:54 am Alright, here's some fixes.
Committed at 322b25a4. I also unilaterally removed the use of _wfopen_s, commit 95e6819f.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

SarahWalker wrote: Sun 15 Nov, 2020 9:03 am
shermanp wrote: Sun 15 Nov, 2020 6:54 am Alright, here's some fixes.
Committed at 322b25a4. I also unilaterally removed the use of _wfopen_s, commit 95e6819f.
Thank you Sarah. I originally used _wfopen(), so that change is fine by me. Those '_s' functions were something I kind of glossed over during my review of the code. I know for next time!
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [Patch] New VHD support... now with added dynamics

Post by SarahWalker »

Image not closing should be fixed in d6bf0bae.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

I have one more little tweak.

My apologies for not including it earlier, it's something I meant to do, and it slipped my mind.

This diff replaces the "remember to format" message with a "don't open/modify the parent" message when creating a differencing VHD image. The former message isn't very relevant for differencing images, but I feel the warning is much more important.

Diff is against latest PCem commit (5c20fe7).

Code: Select all

diff --git a/src/wx-config.c b/src/wx-config.c
index 2421d67..9f24e2c 100644
--- a/src/wx-config.c
+++ b/src/wx-config.c
@@ -1842,9 +1842,13 @@ static int hdnew_dlgproc(void* hdlg, int message, INT_PARAM wParam, LONG_PARAM l
                                                 return TRUE;
                                         }
                                 }
+                                wx_messagebox(hdlg, "Differencing VHD image created.\n\n"
+                                        "WARNING: Do not open or modify the parent image(s) while this file exists.", "PCem", WX_MB_OK);
+                        }
+                        if (hd_format != 3)
+                        {
+                                wx_messagebox(hdlg, "Drive created, remember to partition and format the new drive.", "PCem", WX_MB_OK);
                         }
-
-                        wx_messagebox(hdlg, "Drive created, remember to partition and format the new drive.", "PCem", WX_MB_OK);
 
                         wx_enddialog(hdlg, 1);
                         return TRUE;

Laucian
Posts: 11
Joined: Mon 28 Nov, 2016 6:49 pm

Re: [Patch] New VHD support... now with added dynamics

Post by Laucian »

I finally had more time to test out the changes. It compiles fine now.
Under Linux I am experiencing

Code: Select all

Segmentation fault (core dumped)
whenever I create a new VHD or attach an existing one to a machine. Editing the config file to use an older VHD built on the earlier version of the PCem patch gives the same results. IMG files work fine.
I ran PCem through Valgrind to diagnose the issue and it worked through there oddly enough.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

Laucian wrote: Wed 18 Nov, 2020 2:47 am I finally had more time to test out the changes. It compiles fine now.
Under Linux I am experiencing

Code: Select all

Segmentation fault (core dumped)
whenever I create a new VHD or attach an existing one to a machine.
Nooooo, not the dreaded segfault :(

Let's see if I can replicate.

EDIT: yes I can. A bit more Linux testing was obviously in order. You have my humble apologies.
Now to see if I can figure out what's going wrong.

EDIT2: Good news, it's not crashing because of a memory related error. Bad news, it's segfaulting when trying to call fseeko64. Large file support makes my head hurt :( I may not have set a correct compiler flag or something? I'll investigate further.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

Ok, here's a patch that should correct the segfaults on Linux.

Turns out I had forgotten to set the large file macro's while attempting to use fseeko64 and friends.

I decided in the end to switch to using fseeko, and setting _FILE_OFFSET_BITS 64 instead. It seems to work alright with both Linux and mingw-w64.

Code: Select all

diff --git a/src/minivhd/cwalk.c b/src/minivhd/cwalk.c
index 265de0f..f0c4842 100644
--- a/src/minivhd/cwalk.c
+++ b/src/minivhd/cwalk.c
@@ -1,3 +1,6 @@
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <assert.h>
 #include <ctype.h>
 #include <stdarg.h>
diff --git a/src/minivhd/minivhd_convert.c b/src/minivhd/minivhd_convert.c
index 231e0f9..1de6f46 100644
--- a/src/minivhd/minivhd_convert.c
+++ b/src/minivhd/minivhd_convert.c
@@ -1,3 +1,6 @@
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <stdbool.h>
 #include <stdlib.h>
 #include <stdio.h>
diff --git a/src/minivhd/minivhd_create.c b/src/minivhd/minivhd_create.c
index 9e34ece..c47c7d8 100644
--- a/src/minivhd/minivhd_create.c
+++ b/src/minivhd/minivhd_create.c
@@ -1,3 +1,6 @@
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <assert.h>
 #include <stdlib.h>
 #include <stdio.h>
diff --git a/src/minivhd/minivhd_io.c b/src/minivhd/minivhd_io.c
index 8e9172e..ddd1717 100644
--- a/src/minivhd/minivhd_io.c
+++ b/src/minivhd/minivhd_io.c
@@ -3,6 +3,9 @@
  * \brief Sector reading and writing implementations
  */
 
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <stdlib.h>
 #include <string.h>
 #include "minivhd_internal.h"
diff --git a/src/minivhd/minivhd_manage.c b/src/minivhd/minivhd_manage.c
index 75e0953..e9473c0 100644
--- a/src/minivhd/minivhd_manage.c
+++ b/src/minivhd/minivhd_manage.c
@@ -2,7 +2,9 @@
  * \file
  * \brief VHD management functions (open, close, read write etc)
  */
-
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdint.h>
diff --git a/src/minivhd/minivhd_struct_rw.c b/src/minivhd/minivhd_struct_rw.c
index 66f2289..c77fa60 100644
--- a/src/minivhd/minivhd_struct_rw.c
+++ b/src/minivhd/minivhd_struct_rw.c
@@ -2,7 +2,9 @@
  * \file
  * \brief Header and footer serialize/deserialize functions
  */
-
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdint.h>
diff --git a/src/minivhd/minivhd_struct_rw.h b/src/minivhd/minivhd_struct_rw.h
index 0b1e018..ee49bb6 100644
--- a/src/minivhd/minivhd_struct_rw.h
+++ b/src/minivhd/minivhd_struct_rw.h
@@ -1,5 +1,5 @@
 #ifndef MINIVHD_STRUCT_RW_H
-#define minivhd_struct_rw
+#define MINIVHD_STRUCT_RW_H
 
 #include "minivhd_internal.h"
 
diff --git a/src/minivhd/minivhd_util.c b/src/minivhd/minivhd_util.c
index d8f44ca..edede96 100644
--- a/src/minivhd/minivhd_util.c
+++ b/src/minivhd/minivhd_util.c
@@ -2,7 +2,9 @@
  * \file
  * \brief Utility functions
  */
-
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <errno.h>
 #include <stdlib.h>
 #include <stdbool.h>
@@ -152,7 +154,7 @@ FILE* mvhd_fopen(const char* path, const char* mode, int* err) {
         }
     }
 #else
-    f = fopen64(path, mode);
+    f = fopen(path, mode);
     if (f == NULL) {
         mvhd_errno = errno;
         *err = MVHD_ERR_FILE;
@@ -254,7 +256,7 @@ int64_t mvhd_ftello64(FILE* stream)
 #ifdef _MSC_VER
     return _ftelli64(stream);
 #else
-    return ftello64(stream);
+    return ftello(stream);
 #endif
 }
 
@@ -263,7 +265,7 @@ int mvhd_fseeko64(FILE* stream, int64_t offset, int origin)
 #ifdef _MSC_VER
     return _fseeki64(stream, offset, origin);
 #else
-    return fseeko64(stream, offset, origin);
+    return fseeko(stream, offset, origin);
 #endif
 }

Laucian
Posts: 11
Joined: Mon 28 Nov, 2016 6:49 pm

Re: [Patch] New VHD support... now with added dynamics

Post by Laucian »

I just tested and it is working fine under Linux now. As usual cross-platform support is a bit of a pain.
Thank you very much for the work on the patch, differencing images are such a lifesaver when it comes to managing Windows 9x and a large software library.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

Laucian wrote: Wed 18 Nov, 2020 8:23 pm I just tested and it is working fine under Linux now. As usual cross-platform support is a bit of a pain.
Thank you very much for the work on the patch, differencing images are such a lifesaver when it comes to managing Windows 9x and a large software library.
Thanks. If you notice any issues with differencing disks, please let me know. It's probably the bit I'm least confident about. Especially since it has to do a lot of file path manipulation, which is an even bigger cross platform pain than the large file support was. Sometimes I hate C...

And just a word of warning, don't modify the parent image(s) when a differencing image exists. There's some protection built in to detect it, but it's based on file modification time, so rather fragile. There is a reason Microsoft created the VHDX format.
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [Patch] New VHD support... now with added dynamics

Post by SarahWalker »

With the most recent patch it no longer builds for me on mingw. ftello and fseeko don't exist.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

SarahWalker wrote: Wed 18 Nov, 2020 8:47 pm With the most recent patch it no longer builds for me on mingw. ftello and fseeko don't exist.
Whyyyyyyyyyyyyy!!!!!!

Ok, looks like I need an ifdef for mingw specifically to separate from mingw-w64. They obviously can't be treated the same.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

Alright. Hopefully third times the charm...

Now, MSVC gets _ftelli64/_fseeki64. MinGW and MinGW-w64 get ftello64/fseeko64. Linux gets large file ftello/fseeko by way of _FILE_OFFSET_BITS and OSX/BSD supposedly default to large file support for ftello/fseeko.

Please let this work.

Code: Select all

diff --git a/src/minivhd/cwalk.c b/src/minivhd/cwalk.c
index 265de0f..f0c4842 100644
--- a/src/minivhd/cwalk.c
+++ b/src/minivhd/cwalk.c
@@ -1,3 +1,6 @@
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <assert.h>
 #include <ctype.h>
 #include <stdarg.h>
diff --git a/src/minivhd/minivhd_convert.c b/src/minivhd/minivhd_convert.c
index 231e0f9..1de6f46 100644
--- a/src/minivhd/minivhd_convert.c
+++ b/src/minivhd/minivhd_convert.c
@@ -1,3 +1,6 @@
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <stdbool.h>
 #include <stdlib.h>
 #include <stdio.h>
diff --git a/src/minivhd/minivhd_create.c b/src/minivhd/minivhd_create.c
index 9e34ece..c47c7d8 100644
--- a/src/minivhd/minivhd_create.c
+++ b/src/minivhd/minivhd_create.c
@@ -1,3 +1,6 @@
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <assert.h>
 #include <stdlib.h>
 #include <stdio.h>
diff --git a/src/minivhd/minivhd_io.c b/src/minivhd/minivhd_io.c
index 8e9172e..ddd1717 100644
--- a/src/minivhd/minivhd_io.c
+++ b/src/minivhd/minivhd_io.c
@@ -3,6 +3,9 @@
  * \brief Sector reading and writing implementations
  */
 
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <stdlib.h>
 #include <string.h>
 #include "minivhd_internal.h"
diff --git a/src/minivhd/minivhd_manage.c b/src/minivhd/minivhd_manage.c
index 75e0953..e9473c0 100644
--- a/src/minivhd/minivhd_manage.c
+++ b/src/minivhd/minivhd_manage.c
@@ -2,7 +2,9 @@
  * \file
  * \brief VHD management functions (open, close, read write etc)
  */
-
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdint.h>
diff --git a/src/minivhd/minivhd_struct_rw.c b/src/minivhd/minivhd_struct_rw.c
index 66f2289..c77fa60 100644
--- a/src/minivhd/minivhd_struct_rw.c
+++ b/src/minivhd/minivhd_struct_rw.c
@@ -2,7 +2,9 @@
  * \file
  * \brief Header and footer serialize/deserialize functions
  */
-
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdint.h>
diff --git a/src/minivhd/minivhd_struct_rw.h b/src/minivhd/minivhd_struct_rw.h
index 0b1e018..ee49bb6 100644
--- a/src/minivhd/minivhd_struct_rw.h
+++ b/src/minivhd/minivhd_struct_rw.h
@@ -1,5 +1,5 @@
 #ifndef MINIVHD_STRUCT_RW_H
-#define minivhd_struct_rw
+#define MINIVHD_STRUCT_RW_H
 
 #include "minivhd_internal.h"
 
diff --git a/src/minivhd/minivhd_util.c b/src/minivhd/minivhd_util.c
index d8f44ca..5bfc599 100644
--- a/src/minivhd/minivhd_util.c
+++ b/src/minivhd/minivhd_util.c
@@ -2,7 +2,9 @@
  * \file
  * \brief Utility functions
  */
-
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
 #include <errno.h>
 #include <stdlib.h>
 #include <stdbool.h>
@@ -152,7 +154,7 @@ FILE* mvhd_fopen(const char* path, const char* mode, int* err) {
         }
     }
 #else
-    f = fopen64(path, mode);
+    f = fopen(path, mode);
     if (f == NULL) {
         mvhd_errno = errno;
         *err = MVHD_ERR_FILE;
@@ -253,8 +255,10 @@ int64_t mvhd_ftello64(FILE* stream)
 {
 #ifdef _MSC_VER
     return _ftelli64(stream);
-#else
+#elif defined(__MINGW32__)
     return ftello64(stream);
+#else /* This should work with linux (with _FILE_OFFSET_BITS), and hopefully OS X and BSD */
+    return ftello(stream);
 #endif
 }
 
@@ -262,8 +266,10 @@ int mvhd_fseeko64(FILE* stream, int64_t offset, int origin)
 {
 #ifdef _MSC_VER
     return _fseeki64(stream, offset, origin);
-#else
+#elif defined(__MINGW32__)
     return fseeko64(stream, offset, origin);
+#else /* This should work with linux (with _FILE_OFFSET_BITS), and hopefully OS X and BSD */
+    return fseeko(stream, offset, origin);
 #endif
 }
 
Laucian
Posts: 11
Joined: Mon 28 Nov, 2016 6:49 pm

Re: [Patch] New VHD support... now with added dynamics

Post by Laucian »

I just tested the latest patch on Linux, everything still works.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

Laucian wrote: Wed 18 Nov, 2020 11:43 pm I just tested the latest patch on Linux, everything still works.
Good to know.

I've got things setup on my Ubuntu box to do some rudimentary testing as well now, so I (hopefully) shouldn't break linux compilation any more.
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [Patch] New VHD support... now with added dynamics

Post by SarahWalker »

Works okay here.
Greatpsycho
Posts: 151
Joined: Tue 22 Mar, 2016 10:03 am
Location: Korea
Contact:

Re: [Patch] New VHD support... now with added dynamics

Post by Greatpsycho »

This patch solves compile errors on CentOS.
Attachments
centos_patches.tar.gz
Patch for solve compile error on CentOS.
(964 Bytes) Downloaded 649 times
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

@Greatpsycho does removing the typedef from minivhd_internal.h and including the minivhd.h header instead work for you? I'd rather avoid the #ifdef if I can.
SA1988
Posts: 274
Joined: Wed 30 Apr, 2014 9:38 am

Re: [Patch] New VHD support... now with added dynamics

Post by SA1988 »

Just tried NT, first part of installation can be done normally, but during boot, I get gibberish during ntldr or unknown hard error on the dynamic VHD I'm using, this doesn't happen with fixed size VHD or plain RAW.
Note that the file system is FAT16, not NTFS, of the dynamic vhd I'm using.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

I can't reproduce this in PCem. I can reproduce it in another emulator. It is not yet known whether this is an underlying MiniVHD issue, or an integration issue.
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [Patch] New VHD support... now with added dynamics

Post by SarahWalker »

shermanp wrote: Wed 18 Nov, 2020 10:16 pm Alright. Hopefully third times the charm...
Committed at 302d05c.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

SA1988 wrote: Fri 20 Nov, 2020 8:00 pm Just tried NT, first part of installation can be done normally, but during boot, I get gibberish during ntldr or unknown hard error on the dynamic VHD I'm using, this doesn't happen with fixed size VHD or plain RAW.
Note that the file system is FAT16, not NTFS, of the dynamic vhd I'm using.
shermanp wrote: Fri 20 Nov, 2020 10:07 pm I can't reproduce this in PCem. I can reproduce it in another emulator. It is not yet known whether this is an underlying MiniVHD issue, or an integration issue.
Ok, this WAS a MiniVHD bug, a fairly nasty one too. BUT it only manifested itself for some types of multi-sector writes, which PCem doesn't appear to do, but if it ever does, would have bit PCem as well. sards3 has authored a fix and is in the set of patches attached.

Patch 0002-Remove-duplicate-typedef-from-minivhd_internal.patch is my preferred alternative to Greatpsycho's two minivhd patches.

There's also a mass EOF fixup, and added some functions to the public header that should have always been there.

Unless any other bugs crop up, I'm hoping this will be it for now.

(Note, I've done individual patches, as well as a all-in-one diff)
Attachments
vhd-fixes-20201122.zip
(8.93 KiB) Downloaded 452 times
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [Patch] New VHD support... now with added dynamics

Post by SarahWalker »

All committed as of 8fa271d.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

SarahWalker wrote: Sat 21 Nov, 2020 8:59 pm All committed as of 8fa271d.
Thank you very much.

Thank you for your patience with this.
Greatpsycho
Posts: 151
Joined: Tue 22 Mar, 2016 10:03 am
Location: Korea
Contact:

Re: [Patch] New VHD support... now with added dynamics

Post by Greatpsycho »

shermanp wrote: Sat 21 Nov, 2020 11:10 am
SA1988 wrote: Fri 20 Nov, 2020 8:00 pm Just tried NT, first part of installation can be done normally, but during boot, I get gibberish during ntldr or unknown hard error on the dynamic VHD I'm using, this doesn't happen with fixed size VHD or plain RAW.
Note that the file system is FAT16, not NTFS, of the dynamic vhd I'm using.
shermanp wrote: Fri 20 Nov, 2020 10:07 pm I can't reproduce this in PCem. I can reproduce it in another emulator. It is not yet known whether this is an underlying MiniVHD issue, or an integration issue.
Ok, this WAS a MiniVHD bug, a fairly nasty one too. BUT it only manifested itself for some types of multi-sector writes, which PCem doesn't appear to do, but if it ever does, would have bit PCem as well. sards3 has authored a fix and is in the set of patches attached.

Patch 0002-Remove-duplicate-typedef-from-minivhd_internal.patch is my preferred alternative to Greatpsycho's two minivhd patches.

There's also a mass EOF fixup, and added some functions to the public header that should have always been there.

Unless any other bugs crop up, I'm hoping this will be it for now.

(Note, I've done individual patches, as well as a all-in-one diff)
There is still compile problem exists on CentOS. Your VHD code uses C99 extensions but it's not enabled by default on CentOS. To solve this problem, configure.ac file must be modified as described below, or all VHD related code must be rewritten to avoid using C99 extensions.
pcem-compile-error.png
pcem-compile-error.png (72.01 KiB) Viewed 16804 times
Old configure.ac

Code: Select all

AC_MSG_CHECKING([whether to enable debugging])
AC_ARG_ENABLE(debug,
          AC_HELP_STRING([--enable-debug], [build debug executable]))
if test "$enable_debug" = "yes"; then
   CFLAGS="-Wall -O0 -g -D_DEBUG -fcommon"
   CXXFLAGS="-Wall -O0 -g -D_DEBUG -fcommon"
   AC_MSG_RESULT([yes])
else
   CFLAGS="-O3 -fcommon"
   CXXFLAGS="-O3 -fcommon"
   AC_MSG_RESULT([no])
fi
New configure.ac

Code: Select all

AC_MSG_CHECKING([whether to enable debugging])
AC_ARG_ENABLE(debug,
          AC_HELP_STRING([--enable-debug], [build debug executable]))
if test "$enable_debug" = "yes"; then
   CFLAGS="-Wall -O0 -g -D_DEBUG -fcommon -std=gnu99"
   CXXFLAGS="-Wall -O0 -g -D_DEBUG -fcommon"
   AC_MSG_RESULT([yes])
else
   CFLAGS="-O3 -fcommon -std=gnu99"
   CXXFLAGS="-O3 -fcommon"
   AC_MSG_RESULT([no])
fi
Attachments
configure.ac.patch
Patch for solve compile error on CentOS.
(558 Bytes) Downloaded 428 times
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

Greatpsycho wrote: Sun 22 Nov, 2020 7:38 am
There is still compile problem exists on CentOS. Your VHD code uses C99 extensions but it's not enabled by default on CentOS. To solve this problem, configure.ac file must be modified as described below, or all VHD related code must be rewritten to avoid using C99 extensions.

pcem-compile-error.png

Old configure.ac

Code: Select all

AC_MSG_CHECKING([whether to enable debugging])
AC_ARG_ENABLE(debug,
          AC_HELP_STRING([--enable-debug], [build debug executable]))
if test "$enable_debug" = "yes"; then
   CFLAGS="-Wall -O0 -g -D_DEBUG -fcommon"
   CXXFLAGS="-Wall -O0 -g -D_DEBUG -fcommon"
   AC_MSG_RESULT([yes])
else
   CFLAGS="-O3 -fcommon"
   CXXFLAGS="-O3 -fcommon"
   AC_MSG_RESULT([no])
fi
New configure.ac

Code: Select all

AC_MSG_CHECKING([whether to enable debugging])
AC_ARG_ENABLE(debug,
          AC_HELP_STRING([--enable-debug], [build debug executable]))
if test "$enable_debug" = "yes"; then
   CFLAGS="-Wall -O0 -g -D_DEBUG -fcommon -std=gnu99"
   CXXFLAGS="-Wall -O0 -g -D_DEBUG -fcommon"
   AC_MSG_RESULT([yes])
else
   CFLAGS="-O3 -fcommon -std=gnu99"
   CXXFLAGS="-O3 -fcommon"
   AC_MSG_RESULT([no])
fi
Really? It's the end of 2020, and there's still compilers which don't compile code from a 20 year old standard out-of-the box? Sheesh.
sards3
Posts: 7
Joined: Tue 02 Apr, 2019 7:55 pm

Re: [Patch] New VHD support... now with added dynamics

Post by sards3 »

My recommendation would be to upgrade your compiler.
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

sards3 wrote: Sun 22 Nov, 2020 10:12 am My recommendation would be to upgrade your compiler.
May not be possible on an old version of CentOS.

I shouldn't think adding -std=gnu99" to configure.ac would cause too many issues though, although maaaybe it would need to be linux or gcc specific?

EDIT: If the only thing the compiler is complaining about is the declaration in the for loop, I could probably change that in MiniVHD.
Greatpsycho
Posts: 151
Joined: Tue 22 Mar, 2016 10:03 am
Location: Korea
Contact:

Re: [Patch] New VHD support... now with added dynamics

Post by Greatpsycho »

shermanp wrote: Sun 22 Nov, 2020 10:25 am
sards3 wrote: Sun 22 Nov, 2020 10:12 am My recommendation would be to upgrade your compiler.
May not be possible on an old version of CentOS.

I shouldn't think adding -std=gnu99" to configure.ac would cause too many issues though, although maaaybe it would need to be linux or gcc specific?

EDIT: If the only thing the compiler is complaining about is the declaration in the for loop, I could probably change that in MiniVHD.
The compiler only complaining about declaration in the for loop. This patch removes declaration in the for loop and solves compile problem on CentOS.
Attachments
minivhd_patches.tar.gz
Patch for remove declaration in the for loop.
(2.34 KiB) Downloaded 440 times
shermanp
Posts: 175
Joined: Sat 18 Feb, 2017 2:09 am

Re: [Patch] New VHD support... now with added dynamics

Post by shermanp »

Greatpsycho wrote: Sun 22 Nov, 2020 11:35 am The compiler only complaining about declaration in the for loop. This patch removes declaration in the for loop and solves compile problem on CentOS.
Feel free to integrate this into PCem Sarah, I've committed the changes to MiniVHD as-is.
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [Patch] New VHD support... now with added dynamics

Post by SarahWalker »

Committed at 0ec9c6e.
Post Reply