Post Reply 
Free42 state file switching, export, and import
08-14-2019, 10:03 PM
Post: #1
Free42 state file switching, export, and import
I'm working on state file switching, export, and import in Free42. It's currently working in the Mac and Windows versions. I put test versions up here: https://thomasokken.com/free42/download/test/

These have a new platform-independent state file format.
The GTK (Linux), iOS, and Android versions will follow over the course of the next few weeks.

Comments welcome. Enjoy!
Find all posts by this user
Quote this message in a reply
08-14-2019, 10:52 PM
Post: #2
RE: Free42 state file switching, export, and import
Thanks Thomas ! You've finally done it ! Can't wait for the Android version Smile
Cheers
Find all posts by this user
Quote this message in a reply
08-15-2019, 04:37 AM
Post: #3
RE: Free42 state file switching, export, and import
Nice! Thank you, Thomas.

I can test it once there is a Linux or Android version.
Find all posts by this user
Quote this message in a reply
08-15-2019, 03:15 PM
Post: #4
RE: Free42 state file switching, export, and import
Btw, could the state file format be compatible / same as emu42 ? Could be useful..
Find all posts by this user
Quote this message in a reply
08-15-2019, 03:49 PM
Post: #5
RE: Free42 state file switching, export, and import
I'm sure that a person who knows the HP-42S through and through could take a memory dump from that machine -- or an Emu42 state file, which I assume is basically the same thing -- and translate it to a Free42 state file. However, I am not that person. I only know the 42S from a user's point of view; I've never looked at the internals or ROM disassembly etc.
Find all posts by this user
Quote this message in a reply
08-15-2019, 04:45 PM
Post: #6
RE: Free42 state file switching, export, and import
(08-15-2019 03:49 PM)Thomas Okken Wrote:  I'm sure that a person who knows the HP-42S through and through could take a memory dump from that machine -- or an Emu42 state file, which I assume is basically the same thing -- and translate it to a Free42 state file. However, I am not that person. I only know the 42S from a user's point of view; I've never looked at the internals or ROM disassembly etc.
Fair enough Thomas, I overlooked that ! Never mind then, .raw files compatibility is enough...
Find all posts by this user
Quote this message in a reply
08-17-2019, 08:07 PM (This post was last modified: 08-17-2019 10:01 PM by Thomas Okken.)
Post: #7
RE: Free42 state file switching, export, and import
Finished & uploaded the Linux version.

The GTK build now works in FreeBSD and Solaris as well; I also uploaded experimental binaries for those platforms.
(FreeBSD 12 and Solaris 11.3, both on x86_64.)
Find all posts by this user
Quote this message in a reply
Yesterday, 09:43 AM
Post: #8
RE: Free42 state file switching, export, and import
Hi Thomas,

Wow, that are many changes to the source! That must have been a lot of time that you invested in such a "small" feature and everything for free and open source! Thank you!

Here are a few remarks after compiling Free42 under Arch Linux:

  1. Arch Linux requires the X11 library as well (LIBS += -lX11). Probably all Linux distributions require it.
  2. At least under Arch Linux -Wno-narrowing is necessary. As you've added it to the FreeBSD and SunOS builds, can you please add it to the normal Linux build as well? I think there are no downsides.
  3. Speaking of compile options: Adding -Wno-missing-braces -Wno-unused-but-set-variable -Wno-unused-variable removes hundreds of warnings.
  4. The buffer for the path generation in gtk/shell_main.cc is too small:
    Code:
    shell_main.cc:384:57: warning: '%s' directive output may be truncated writing up to 255 bytes into a region of size between 0 and 255 [-Wformat-truncation=]
      384 |         snprintf(core_state_file_name, FILENAMELEN, "%s/%s.f42", free42dirname, state.coreName);
          |                                                         ^~                      ~~~~~~~~~~~~~~
    shell_main.cc:384:17: note: 'snprintf' output between 6 and 516 bytes into a destination of size 256
      384 |         snprintf(core_state_file_name, FILENAMELEN, "%s/%s.f42", free42dirname, state.coreName);
          |         ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~​~~~~~~~~~~~~
    You are trying to put two strings with a length of up to 255 characters in a buffer of only 255 characters. There are several places with this issue.
  5. There are some misleading-indentation warnings like
    Code:
    core_keydown.cc: In function 'void keydown_normal_mode(int, int)':
    core_keydown.cc:1821:18: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     1821 |             else if (prgms[current_prgm].text[pc] != CMD_END)
          |                  ^~
    core_keydown.cc:1823:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
     1823 |                 prgm_highlight_row = 1;
          |                 ^~~~~~~~~~~~~~~~~~
Find all posts by this user
Quote this message in a reply
Yesterday, 11:37 AM
Post: #9
RE: Free42 state file switching, export, and import
Hi Sammy,

Thank you for your feedback!

The platform-independent state files did require a lot of code changes, but that was a feature I've wanted for a long time, and doing it the way I ended up doing wasn't that bad, it took about a day and a half in the end. And it gave me an opportunity to simplify the core/shell interface a bit. I'm pretty happy with the result.

The state file management dialog is a lot more work, because that's all shell code, so it has to be done five times. Three done, two to go! I'll work on the Android version today, that should be a bit less work that the previous three. For the iOS version, I'll probably be able to reuse a lot of the code I wrote for the MacOS version, but that's going to have to wait, there's no way I'll be able to finish both this weekend.

My comments on your comments follow below.

Thomas

(Yesterday 09:43 AM)SammysHP Wrote:  1. Arch Linux requires the X11 library as well (LIBS += -lX11). Probably all Linux distributions require it.

The X11 library is always required, but what varies is whether you have to tell the linker about it or not. I do my Linux builds in Ubuntu 12.04, and -lX11 is not needed there. I'll have to check what happens when I do add it there; I don't remember whether I left it off out of misguided command-line minimalism or because of a warning.

(Yesterday 09:43 AM)SammysHP Wrote:  2. At least under Arch Linux -Wno-narrowing is necessary. As you've added it to the FreeBSD and SunOS builds, can you please add it to the normal Linux build as well? I think there are no downsides.

The downsides are error messages from compilers that don't recognize -Wno-narrowing. Big Grin

(Yesterday 09:43 AM)SammysHP Wrote:  3. Speaking of compile options: Adding -Wno-missing-braces -Wno-unused-but-set-variable -Wno-unused-variable removes hundreds of warnings.

Argh. In Ubuntu 12.04, there are zero warnings (there are some in the Intel library but none in Free42 itself). Newer compilers warn about more things, and some of those warnings are idiotic in my opinion (like the ones about adding parentheses in expressions like a && b || c && d) and others are about things that used to be common, even unavoidable, in C programming, and for which better alternatives sometimes exist in newer versions of the C and C++ standards. Unfortunately, bringing the source code up to date with those new standards is a big task, especially considering all the regression testing that would be required, so I just end up adding -Wno-annoying-warning-du-jour options in an endless game of whac-a-mole. And what makes this especially annoying is that every revision of gcc/llvm/clang on every distribution and every OS has its own set of warnings, and you can't just turn them all off, because turning off a warning that a compiler doesn't recognize, is an error.
See also: item 2, above.

(Yesterday 09:43 AM)SammysHP Wrote:  4. The buffer for the path generation in gtk/shell_main.cc is too small [...]
You are trying to put two strings with a length of up to 255 characters in a buffer of only 255 characters. There are several places with this issue.

I don't think it's an issue in practice, because who actually uses directories nested that deep and filenames that long that you end up with absolute paths of more than 255 characters? My main reason for using snprintf is that in the unlikely event of such extremely long paths occurring, the code would at least fail gracefully rather than crash.

Still, it's not exactly elegant, and a few days ago, I went through the Windows version and eliminated all fixed-size strings wherever I could, replacing them with std::string. It turned out to require a ridiculous number of code changes, and to re-test everything because those fixed-length strings are everywhere, and then I would have to repeat that whole miserable exercise in the GTK, MacOS, and iOS versions as well. (The Android shell is Java so no fixed-length strings there, praise be to James Gosling and the 1990s Sun Microsystems managers who recognized the value of his work!) So in the end I backed out those changes. Maybe I'll re-apply them in some indeterminate future, when I'm sufficiently bored and when I feel up to the additional challenge of making the Windows version fully Unicode-compatible at the same time. Smile

Sorry about the rant! TL;DR It's not something to worry about, but the compiler is too stupid to know that, and I'm unable to deal with all warnings myself; that task falls on downstream package maintainers. See also items 2 and 3...

(Yesterday 09:43 AM)SammysHP Wrote:  5. There are some misleading-indentation warnings

Misleading indentation is misleading! I'll take a look and fix the source where I find them, at least where I agree with the warnings. The indentation in the example you gave is indeed wrong.
Find all posts by this user
Quote this message in a reply
Yesterday, 11:53 AM
Post: #10
RE: Free42 state file switching, export, and import
(Yesterday 11:37 AM)Thomas Okken Wrote:  
(Yesterday 09:43 AM)SammysHP Wrote:  1. Arch Linux requires the X11 library as well (LIBS += -lX11). Probably all Linux distributions require it.

The X11 library is always required, but what varies is whether you have to tell the linker about it or not. I do my Linux builds in Ubuntu 12.04, and -lX11 is not needed there. I'll have to check what happens when I do add it there; I don't remember whether I left it off out of misguided command-line minimalism or because of a warning.

Since Ubuntu 12.04 went EOL over 2 years ago, have you considered building it on something more modern? At least to pick up the problems reported by later versions of gcc.

Quote:
(Yesterday 09:43 AM)SammysHP Wrote:  2. At least under Arch Linux -Wno-narrowing is necessary. As you've added it to the FreeBSD and SunOS builds, can you please add it to the normal Linux build as well? I think there are no downsides.

The downsides are error messages from compilers that don't recognize -Wno-narrowing. Big Grin

The other option is to fix the warnings. Smile

— Ian Abbott
Find all posts by this user
Quote this message in a reply
Yesterday, 12:03 PM
Post: #11
RE: Free42 state file switching, export, and import
(Yesterday 11:53 AM)ijabbott Wrote:  
(Yesterday 11:37 AM)Thomas Okken Wrote:  The X11 library is always required, but what varies is whether you have to tell the linker about it or not. I do my Linux builds in Ubuntu 12.04, and -lX11 is not needed there. I'll have to check what happens when I do add it there; I don't remember whether I left it off out of misguided command-line minimalism or because of a warning.

Since Ubuntu 12.04 went EOL over 2 years ago, have you considered building it on something more modern? At least to pick up the problems reported by later versions of gcc.

The advantage of building on an old distribution is that the binaries I create work just about everywhere. If I were to build on a newer distro, that would probably cause incompatibilities with older ones.

(Yesterday 11:53 AM)ijabbott Wrote:  
(Yesterday 11:37 AM)Thomas Okken Wrote:  The downsides are error messages from compilers that don't recognize -Wno-narrowing. Big Grin

The other option is to fix the warnings. Smile

I think you missed the part of how much work that would entail. I can think of many more enjoyable and/or productive ways to spend my time!
Find all posts by this user
Quote this message in a reply
Yesterday, 12:45 PM
Post: #12
RE: Free42 state file switching, export, and import
You are right, Thomas. Each new version of gcc brings new features and checks for common issues that were discovered in the meantime or just considered helpful. Sometimes changes aren't even fully backwards compatible without additional options. Additionally every distribution might define a different set of default options, which makes it even more complicated. I work(ed) with all major versions from 5 to 9 and have seen many unexpected things.

Regarding the warning: An unknown -Wno-* option should not produce an error.

https://gcc.gnu.org/wiki/FAQ#wnowarning Wrote:Since GCC 4.4, and as explained in the GCC manual: when an unrecognized warning option is requested (-Wunknown-warning), GCC emits a diagnostic stating that the option is not recognized. However, if the -Wno- form is used, the behavior is slightly different: no diagnostic is produced for -Wno-unknown-warning unless other diagnostics are being produced. This allows the use of new -Wno- options with old compilers, but if something goes wrong, the compiler warns that an unrecognized option is present.

On the other hand I have no problems with patching your makefile as a package maintainer. But it causes additional work for me and people who want to compile Free42 directly from source.

(Yesterday 11:37 AM)Thomas Okken Wrote:  
(Yesterday 09:43 AM)SammysHP Wrote:  4. The buffer for the path generation in gtk/shell_main.cc is too small [...]
You are trying to put two strings with a length of up to 255 characters in a buffer of only 255 characters. There are several places with this issue.

I don't think it's an issue in practice, because who actually uses directories nested that deep and filenames that long that you end up with absolute paths of more than 255 characters? My main reason for using snprintf is that in the unlikely event of such extremely long paths occurring, the code would at least fail gracefully rather than crash.

Here I like to disagree. First, "who actually…" caused a lot of trouble in computer history. Second, you know that this part of the source does not work with valid input in some circumstances, but you ignore it. You don't even check the return value of snprintf(). The code silently fails and (hopefully) an error is produced at different place (when the program tries to read the file). Such long paths are unlikely, but allowed.
In the end this is not a big issue that can cause all kinds of problems (except that loading of the file might fail sometimes), it is just not clean code which could have been avoided.
Find all posts by this user
Quote this message in a reply
Yesterday, 02:57 PM (This post was last modified: Yesterday 03:00 PM by Thomas Okken.)
Post: #13
RE: Free42 state file switching, export, and import
(Yesterday 12:45 PM)SammysHP Wrote:  Regarding the warning: An unknown -Wno-* option should not produce an error.

https://gcc.gnu.org/wiki/FAQ#wnowarning Wrote:Since GCC 4.4, and as explained in the GCC manual: when an unrecognized warning option is requested (-Wunknown-warning), GCC emits a diagnostic stating that the option is not recognized. However, if the -Wno- form is used, the behavior is slightly different: no diagnostic is produced for -Wno-unknown-warning unless other diagnostics are being produced. This allows the use of new -Wno- options with old compilers, but if something goes wrong, the compiler warns that an unrecognized option is present.

Sweet! I did not know that. Even my Ubuntu 12.04 build VM has a sufficiently recent gcc for that (4.6.3 -- whew).
It doesn't mind the redundant -lX11, either. OK, I'll clean up those two things in the Makefile (and fix the indentation where appropriate).

(Yesterday 12:45 PM)SammysHP Wrote:  
(Yesterday 11:37 AM)Thomas Okken Wrote:  I don't think it's an issue in practice, because who actually uses directories nested that deep and filenames that long that you end up with absolute paths of more than 255 characters? My main reason for using snprintf is that in the unlikely event of such extremely long paths occurring, the code would at least fail gracefully rather than crash.

Here I like to disagree. First, "who actually…" caused a lot of trouble in computer history.

Sure, but consider the specific situation at hand. Free42 is not a backup daemon that traverses entire filesystems and thus would absolutely have to be able to handle long paths. Free42 is an interactive program, and all the files it deals with are in $HOME/.free42 or in whatever directory the user specifically chooses. Honestly, who is going to exceed 255 characters that way?

(Yesterday 12:45 PM)SammysHP Wrote:  Second, you know that this part of the source does not work with valid input in some circumstances, but you ignore it. You don't even check the return value of snprintf(). The code silently fails and (hopefully) an error is produced at different place (when the program tries to read the file). Such long paths are unlikely, but allowed.
In the end this is not a big issue that can cause all kinds of problems (except that loading of the file might fail sometimes), it is just not clean code which could have been avoided.

As I said before, the rationale for using snprintf is to avoid crashes, not to make sure extremely long paths actually work. The Windows version doesn't even use snprintf, because it's not provided in Visual C++, at least not the versions I've used. The better is always the enemy of the good, and I'd rather have a program that can be made to malfunction or even crash when trying to use it in extremely remote corners of the filesystem, than not have the program at all. Smile
Find all posts by this user
Quote this message in a reply
Yesterday, 04:29 PM (This post was last modified: Yesterday 04:31 PM by Thomas Okken.)
Post: #14
RE: Free42 state file switching, export, and import
OK, I made those changes to gtk/Makefile, and fixed the indentation warnings in core_display.cc and core_keydown.cc.

Code:
/Users/thomas/xcode/free42/gtk $ svn diff -r 2932 -x -w
Index: Makefile
===================================================================
--- Makefile    (revision 2932)
+++ Makefile    (working copy)
@@ -20,6 +20,11 @@
      -Wno-parentheses \
      -Wno-write-strings \
      -Wno-sign-compare \
+     -Wno-narrowing \
+     -Wno-constant-conversion \
+     -Wno-sometimes-uninitialized \
+     -Wno-format-truncation \
+     -Wno-unknown-warning-option \
      -g \
      -I/usr/X11R6/include \
      $(shell pkg-config --cflags gtk+-2.0) \
@@ -36,26 +41,14 @@
      -D_WCHAR_T_DEFINED
 
 LDFLAGS = -L/usr/X11R6/lib
-LIBS = gcc111libbid.a $(shell pkg-config --libs gtk+-2.0) -lXmu
+LIBS = gcc111libbid.a $(shell pkg-config --libs gtk+-2.0) -lXmu -lX11
 
-ifeq "$(shell uname -s)" "Linux"
-LDFLAGS += -Wl,--hash-style=both
-LIBS += -lpthread
-endif
-
 ifeq "$(shell uname -s)" "FreeBSD"
-CFLAGS += -Wno-narrowing -Wno-constant-conversion -Wno-sometimes-uninitialized
-LIBS += -lX11
 ifdef AUDIO_ALSA
 LIBS += -ldl
 endif
 endif
 
-ifeq "$(shell uname -s)" "SunOS"
-CFLAGS += -Wno-narrowing -Wno-misleading-indentation
-LIBS += -lX11
-endif
-
 ifneq "$(findstring 6162,$(shell echo ab | od -x))" ""
 CFLAGS += -DF42_BIG_ENDIAN -DBID_BIG_ENDIAN
 endif
/Users/thomas/xcode/free42/gtk $

I also had to add -Wno-unknown-warning-option, because clang does still complain about unknown -Wno-foo options. But at least you can tell it not to!

And I removed the -Wl,--hash-style=both from the Linux build. That was a holdover from when I was still using Fedora 6, and I doubt anyone still needs that.

I'll post an updated tarball shortly. No sense in waiting for the new Android build; it is hot and humid here today, and I have no energy, so while I may tinker with the state file management window for Android a bit, there is no way that work is getting completed this weekend.
Find all posts by this user
Quote this message in a reply
Post Reply 




User(s) browsing this thread: 1 Guest(s)