Re: [hatari-devel] opened filehandles not saved/restored in memory snapshot with gemdos hd ?

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Le 15/12/2018 à 01:51, Eero Tamminen a écrit :
Hi Nicolas,

Attached is a patch showing what I think GEMDOS file handle
save & restore (with dummy DTA handling) would look like.

I've only tested that it builds, and that things don't explode when
using save & restore when there are *no* open file handles
(according to "info gemdos" in debugger).

Can you test the rest?



Hi

I tested this patch and it correctly saved/restored the opened filehandler when streaming an MP2 file from disk, so this looks good to go in main repo.

but I had some warning at compilation :

src/gemdos.c: Dans la fonction « restore_file_handle_info »:
src/gemdos.c:884:91: warning: format « %lld » attend un argument de type « long long int » mais l'argument 5 a le type « off_t » {alias « long int »} [-Wformat=] Log_Printf(LOG_WARN, "GEMDOS '%s' handle %d cannot be restored, seek to saved offset %lld failed for: %s\n",

                ~~~^

                %ld
       handle->szMode, i, offset, handle->szActualName);
~~~~~~

My system is linux 64 bit, maybe "offset" size differs on 32 bit system if that's what you're using ?

More serious problem is when compiling for windows : the mtime part of the save/restore does not compile :

/home/npomarede/src/hatari-int/src/gemdos.c: In function 'save_file_handle_info': /home/npomarede/src/hatari-int/src/gemdos.c:828:17: error: 'struct _stati64' has no member named 'st_mtim'; did you mean 'st_mtime'?
   mtime = fstat.st_mtim; /* modification time */
                 ^~~~~~~
                 st_mtime
/home/npomarede/src/hatari-int/src/gemdos.c:835:17: error: 'struct _stati64' has no member named 'st_mtim'; did you mean 'st_mtime'?
   mtime = fstat.st_mtim;
                 ^~~~~~~
                 st_mtime
/home/npomarede/src/hatari-int/src/gemdos.c: In function 'restore_file_handle_info': /home/npomarede/src/hatari-int/src/gemdos.c:875:21: error: 'struct _stati64' has no member named 'st_mtim'; did you mean 'st_mtime'?
  if (memcmp(&(fstat.st_mtim), &mtime, sizeof(mtime)) != 0)
                     ^~~~~~~
                     st_mtime
/home/npomarede/src/hatari-int/src/gemdos.c:884:90: warning: unknown conversion type character 'l' in format [-Wformat=] Log_Printf(LOG_WARN, "GEMDOS '%s' handle %d cannot be restored, seek to saved offset %lld failed for: %s\n",

                  ^
/home/npomarede/src/hatari-int/src/gemdos.c:884:106: warning: format '%s' expects argument of type 'char *', but argument 5 has type 'off_t' {aka 'long long int'} [-Wformat=] Log_Printf(LOG_WARN, "GEMDOS '%s' handle %d cannot be restored, seek to saved offset %lld failed for: %s\n",

                                 ~^

                                 %I64d
       handle->szMode, i, offset, handle->szActualName);
~~~~~~ /home/npomarede/src/hatari-int/src/gemdos.c:884:24: warning: too many arguments for format [-Wformat-extra-args] Log_Printf(LOG_WARN, "GEMDOS '%s' handle %d cannot be restored, seek to saved offset %lld failed for: %s\n",

^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I'm not sure this can be fixed easily if we want to maintain a common code for linux/windows (and mayb osx too, I don't know if your patch compiles OK under OSX).

Also, I don't think saving / checking mtime at restore is really needed. If someone decides to modify the file on disk after it was part of a memory snapshot it would still work if filesze doesn't change for example. I see your point to check this and warn the user, but I'm not sure this part of the patch should be kept if it causes too many compilation difference on various OSes.

Nicolas



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/