Programming an OS/2 App with AI

[Martin Iturbide]

Hello

I was playing with rest of Bidirectional Language Support DLLs files.
BDCALL32, BDIME, IMP, PMBIDI, THAILIB compiles. I had replaced those on ArcaOS and it still boots and I haven't found any issues.

BDWPCLS has WPS classes and I haven't compiled it yet. I need to install the required tools.

Regards

[Dave Yeo]

Real test would be to try on a bidi system. Personally, I've never done anything with bidi.
David A. has the toolkit on his site IIRC.

[Martin Iturbide]

Hello

First of all, I'm sorry if this is crazy or useless, I'm just doing it as an educational experiment.

With Claude Code I did some replacements of some single .exe that are required for OS/2 Warp 4.52 to boot.
CHKDSK32, HARDERR and SORT are compilable with openwatcom and ArcaOS 5.1.2 boots with it on an experimental VM.
-- https://github.com/OS2World/OS2-API-Replacement-Experiment/tree/main/BIN

I'm choosing the files based on my old experiment call cWarp where I determined the minium files required for OS/2 to boot with GUI, Network and sound support.
-- https://wiki.os2world.com/index.php?title=Required_Replacements

Regards

[Martin Iturbide]

Hello

Just trying some more experiments with Claude AI.

Prompt:

Please check this code. https://github.com/OS2World/UTIL-WPS-Animated_Mouse_Pointer
It is for OS/2 Warp 4.52. Give me any pointers of problems or what do you suggest for improvement.

Reply:

Code: [Select]

Let me fetch the repository contents.

Here is a thorough review of the [UTIL-WPS-Animated_Mouse_Pointer](https://github.com/OS2World/UTIL-WPS-Animated_Mouse_Pointer) codebase:

---

## Code Review Summary

This is a well-structured OS/2 WPS class by Christian Langanke. For its era and purpose it is solid, but there are real issues worth addressing if you want to modernize or maintain it.

---

### Bugs / Potential Crashes

**1. Infinite loop in `CreateTmpFile()` (`mptrutil.c`)**
The retry counter is incremented instead of decremented:
```c
ulMaxTries++;  // BUG: should be ulMaxTries--
```
Under the right failure condition this loops forever.

**2. Memory leak in `CopyFromFileToFile()` (`mptrutil.c`)**
`pvData` is allocated with `malloc()` but never freed before the function returns on any code path.

**3. Partial allocation leaks in `LoadPointerFromWinAnimationFile()` (`cursor.c`)**
The loop allocates multiple buffers per iteration (`pbCursorData`, `pbTargetColorData`, `pbXORMask`). When a mid-loop error triggers `break`, previously allocated buffers from prior iterations are not freed.

---

### Buffer Overflow Risks

These are low-severity for a local desktop utility but worth noting:

| Location | Issue |
|---|---|
| `GetHelpLibName()` | `strcat()` into a fixed buffer without cumulative length check |
| `OpenTmpFile()` | `sprintf(szSearchMask, "%s\\%s*.%s", ...)` — no length guard on inputs |
| `ChangeFilename()` | `strcpy()` into `_MAX_PATH` buffer with no check that source fits |
| `Filespec()` | Returns pointer into input string, relies on caller to bound |

Replace `sprintf`/`strcpy`/`strcat` with `snprintf`/`strncpy`/`strncat` equivalents.

---

### Thread Safety / Race Conditions

**4. Hook global variables unprotected (`mptrhook.c`)**
`hookdataGlobal` and `hmodule` are accessed by both the hook callback (`InputHook`) and the install/release functions without any mutex or critical section. On SMP-capable OS/2 (or ArcaOS) this is a real hazard.

**5. Non-atomic timer restart in `mptranim.c`**
The pattern `WinStopTimer` → `WinStartTimer` is not atomic. A concurrent animation update between the two calls can result in a dangling timer ID.

**6. Silent timeout on mutex in `mptranim.c`**
`REQUEST_DATA_ACCESS_TIMEOUT` failures are swallowed silently with a `break`, leaving shared state in an undefined/inconsistent condition. At minimum these should log an error or set a flag.

---

### Design / Maintainability

**7. `MEMCOPY` macro with embedded pointer arithmetic (`cursor.c`)**
```c
#define MEMCOPY(pbPointerData, src, len) \
    memcpy(pbPointerData, src, len); \
    pbPointerData += len;
```
This is a multi-statement macro without braces, which will silently misbehave inside an `if` without braces. It also mutates the pointer argument as a side effect, making it hard to reason about buffer positions.

**8. Strict version string check in `mptrhook.c`**
`SetHooks()` validates a version string to match `__VERSION__` at hook install time. Any rebuild with a changed version string (even a minor bump) will silently refuse to install the hook without a clear error to the user.

**9. No diagnostic/logging infrastructure**
Error paths throughout just `break` or `return rc` with no tracing. Adding even a simple `DPRINTF` macro conditioned on `DEBUG` would make future debugging much easier.

---

### Build System

**10. `makefile` uses `sed` to generate variant source files**
`wpwani.sed` and `wpand.sed` generate alternate implementations from base files. This is clever but fragile — the generated files are not in the repo, and build failures produce confusing errors. Consider replacing with a shared `.c` + compile-time `#define` approach.

**11. IBM ICC compiler assumed**
The makefile is tightly coupled to IBM VisualAge C++ (`icc`). If you want to build with GCC/clang (e.g., under ArcaOS with GCC), the flags (`/Ge-`, `/O+`, etc.) need a conditional block.

---

### Suggestions for Improvement

1. Fix the `ulMaxTries++` bug — it is the most likely to cause a real hang.
2. Add `free(pvData)` in `CopyFromFileToFile()`.
3. Wrap hook globals in a `HMTX` mutex.
4. Replace bare `sprintf`/`strcpy` with length-safe versions.
5. Make the version check in `mptrhook.c` emit a meaningful `WinMessageBox` error rather than silently failing.
6. Add a `DPRINTF`/`TRACE` macro for debug builds.
7. Consider GCC compatibility flags in the makefile for ArcaOS users.

---

Overall the code is competent 1990s OS/2 systems programming. The most actionable fixes are the infinite-loop bug (#1), the memory leak (#2), and the missing mutex on hook globals (#4).
Regards