misc patches to plugins:

[sezero]

• add OpenWatcom makefile to build Win32 CoolEdit plugin.
• use prototyped function ptrs instead of an opaque FARPROC (same as https://github.com/dbry/WavPack/pull/140, but for winamp plugin.)
• winamp/wa_ipc.h: replace UNREFERENCED_PARAMETER(item) with (void)(item) win32 sdk defines UNREFERENCED_PARAMETER(item) as (item) which generates warnings with other compilers.
• winamp plugin, truncate_here(): watcom clib support

Not adding a watcom makefile for winamp plugin because the result is reported to crash in ticket #146.

(Question: One curious thing in winamp plugin is that its reader passes pointers to FILE* pointers instead just just the FILE* pointers themselves. Any specific reason??)

[dbry]

(Question: One curious thing in winamp plugin is that its reader passes pointers to FILE* pointers instead just just the FILE* pointers themselves. Any specific reason??)

Sorry, was looking in the wrong place! :smile_cat:

[sezero]

Sorry, was looking in the wrong place!

The place is actually right :)

I'm referring to passing &out->wv_file instead of just out->wv_file And the reader functions, rightfully, dereference their void* id param to get the FILE* pointer like *(FILE**)id because of that instead of just (FILE*)id.

[dbry]

Sorry, was looking in the wrong place!

The place is actually right :)

I'm referring to passing &out->wv_file instead of just out->wv_file And the reader functions, rightfully, dereference their void* id param to get the FILE* pointer like *(FILE**)id because of that instead of just (FILE*)id.

Okay, so in the Cooledit filter it's required to pass a pointer to the struct because there are things modified in there and, for instance, we keep track of the size of the first block written (which we sometimes have to read and rewrite).

As for the winamp plugin, it does look like the extra level of indirection is unneeded. My guess is that at some point in the past that was a struct, or I planned to change it to a struct for some reason (debugging perhaps).

One thing I notice is that we are not using the close callback, and so maybe at some point in the past I was using that and setting the file pointer to NULL to indicate a closed file (which of course would be impossible if passing the file pointer itself). Note that having the extra indirection allows us to distinguish between a non-existent file (an unused wvc, for example) and a file that we've closed. Short answer: I have no idea! :smiley:

[sezero]

Short answer: I have no idea!

Heh :)

[dbry]

Tested by building both plugins on MSVC and verifying functionality. Thanks!

[sezero]

One nit: In truncate_here() you probably want to return -1 instead of 0 if file is NULL. I missed noticing that in the PR. Nothing big, just for better correctness (I don't know if you check the result of truncate_here, but...)

diff --git a/winamp/in_wv.c b/winamp/in_wv.c
index dec5408..a07fb51 100644
--- a/winamp/in_wv.c
+++ b/winamp/in_wv.c
@@ -1298,7 +1298,7 @@ static int truncate_here (void *id)
         return _chsize_s (_fileno (file), curr_pos);
         #endif
     }
-    return 0;
+    return -1;
 }

 static WavpackStreamReader64 freader = {

[dbry]

One nit: In truncate_here() you probably want to return -1 instead of 0 if file is NULL. I missed noticing that in the PR. Nothing big, just for better correctness (I don't know if you check the result of truncate_here, but...)

diff --git a/winamp/in_wv.c b/winamp/in_wv.c
index dec5408..a07fb51 100644
--- a/winamp/in_wv.c
+++ b/winamp/in_wv.c
@@ -1298,7 +1298,7 @@ static int truncate_here (void *id)
         return _chsize_s (_fileno (file), curr_pos);
         #endif
     }
-    return 0;
+    return -1;
 }

 static WavpackStreamReader64 freader = {

I'm not too concerned about that. I don't think that could ever get called with NULL file, and if it did somehow then I'm not sure whether the correct result would be succeed or fail (other than not crashing). In the other places I have those reader callbacks I don't even check for NULL.

However, seeing that function reminded me of something that will break. Since this is now being built on MINGW, I should probably put in this fix. I also reported this to mingw here, but they don't seem too concerned (or that's not the best place for it).

[sezero]

I also reported this to mingw here, but they don't seem too concerned (or that's not the best place for it).

The bugs tracker unfortunately gets far much less attention than it deserves. It's best that you report the issue to the mingw-w64-public mailing list: https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/ (subscribing to it is required before posting, IIRC.)

[dbry]

And they'll probably want me to try it on the very latest (I'm on 9.3) which is not always easy when you're on the latest available for your distro.