GSTrans/SetVarVal/ReadVarVal in C, please!

I’d really appreciate a re-write of GSTrans/SetVarVal/ReadVarVal written in such a way that it copies the strings (or numbers, or code) into malloc’d blocks of memory. No need for memory management or GSInit/GSRead. I should be able to re-factor that into a multi-processor friendly implementation.

This is as far as I got yesterday (just reading the PRMs):

Types of variable returned from read: 0 String, 1 Number, 2 Macro
 
Types of variable for setting:
  0 String to be GSTrans'd immediately when setting the variable
  1 Number (4 bytes, signed)
  2 String to be GSTrans'd when the variable is read (Macro)
  3 Expression to be evaluated immediately (resulting in number or string)
  4 String that will not be GSTrans'd when setting the variable or reading it
 16 Code (a code fragment that will be executed to read/write the variable)
 
OMG, you can use wildcards in set operations!
 
Wildcards are a problem, there will have to be a flag indicating that a
variable was just found and may be used in another OS_SetVarVal call.
 
Maybe keep a list of current wildcard searches and return a reference to the 
next matching node in the tree. Something like that.
 
TODO:
  Set literal string variable
  Read string variable
  Set number variable
  Read number variable
  Set macro variable
  GSTrans a string containing no variables
  GSTrans a string containing variables
  Read a macro variable

I’m not sure what you’re really asking for here. The interface is the interface, and the implementation can be whatever you like so long as you retain the interface.

In answer to your notes, yes, you can use wildcards in set operations, and by implication in the delete as well. But remember that at any point a variable may fail to be deleted or set. In RISC OS Pyromaniac I initially intended to disallow set operations from using wildcards. It’s not a useful thing to do in general, and almost always it’s dangerous… so… dropped it. But then when I came to implement it, it was simple enough.

Wildcards are just a continuation operation using the prior string supplied as the reference point to find the next variable, and I’m not sure what you mean by having a flag for the variable, though. You cannot retain any kind of list of ‘current searches’ because you inevitably will leak memory if you do so – nobody ever has to continue a wildcard search to completion.

You may have missed it, but the code fragments are copied for the length of the fragment.

As for the GSInit/GSRead, there is not requirement in the external interface that you use GSInit and GSRead at all. They are an implementation detail for the string being GSTrans’d. Similarly (although you didn’t ask about it but it’s related) GSTrans doesn’t have to use GSInit and GSRead to function, which is what it does at the moment. In RISC OS Pyromaniac the implementation is the opposite way around. GSInit/GSRead are implemented in terms of GSTrans – with the obvious excruciatingly slow performance when you’re re-translating a string every single call to GSRead (it’s O(n^2) for the length of the string). But that’s fine because nobody should be using GSInit/GSRead and having slow performance there for a better implementation and experience with the almost exclusively used GSTrans, makes sense. And obviously it makes the ReadVarVal significantly more efficient as it’s not having to pass through the unsafe GSInit/GSRead behaviour. And that’s before you start thinking about the behaviour of GSTrans with each of the different flags set.

Oh, another thing you might have missed… deleting a code variable without using the Code type, merely sets it to an empty string. Deleting the code variable with the type set to Code will actually delete it.

Interestingly my implementation of system variables is only 360 lines long, excluding the SystemRequests interface.

My tests for system variables are somewhat limited – I only have 17 tests. That doesn’t nearly include enough of the possible combinations. Although GSTrans and EvaluateExpression are tested entirely separately. If you produce any particular tests, please drop me a PR at https://github.com/gerph/riscos-tests/ (see s/var_read and s/var_set) or email me, as it would be good to have some more, or some more appropriate ones.

GSTrans/SetVarVal/ReadVarVal in C, please!

That doesn’t look like it should be too hard to do – it looks like it’s about 2000 lines of assembler (s.Arthur2). ~360 lines for a Python version sounds about right.

1. First, do a direct translation (maintaining all the logic, but simplifying internal memory management to just malloc/free or similar)
2. Then set up some fuzz testing or similar, to make sure it’s functionally equivalent to the OS version (maybe by creating a version of s.Arthur2 which can be built as a standalone module which uses a different SWI chunk, or built directly into the test app, so that the fuzzing won’t break the OS or be influenced by the existing vars)
3. Then work on whatever improvements you want to make (threading-friendly, optimisations, etc.), using the testbed to ensure nothing important breaks. If the initial C version has been validated against the kernel version, then any newer C versions can be validated against the original C version, on any platform you like (assuming you do something like handle code variables as if they were a C function pointer)

GSInit/GSRead are implemented in terms of GSTrans

That sounds like a much better way of doing things, yes.

If performance of GSInit/GSRead becomes an issue then it would probably be pretty easy to add a (per-process) cache of the last translated string.

1.5. Write some regular testing to check the behaviour for regressions.

I really cannot believe I have to repeat this over and over again. I appreciate that you suggested fuzz testing, and that’s ideal (yes, I had some fuzz testing for the GSTrans system in RISC OS Pyromaniac because this is an ideal use of it). But having basic, repeatable tests should be the VERY FIRST THING you try to do when trying to reproduce the behaviour of a system.

I would also suggest not doing 1 – ignoring what the current implementation is, and instead just implementing what the API says. Then write your tests and verify those tests against both your implementation and the original implementation.

Finally, decide whether any quirks you have found wish to be carried over. You’ll find those quirks as you write the tests, and then try it out with real things. Trying to directly translate assembler to a new version is inevitably going to make you waste more time reproducing something when you could be just doing the job better properly without being sullied by the manner in which things /were/ implemented.

What I was hoping for was somebody to produce something I could use without me delving into the details!

I’m not sure what you’re really asking for here. The interface is the interface, and the implementation can be whatever you like so long as you retain the interface.

I can reproduce the interface, given the code I asked for. What I need in a multi-threaded preemptive environment are atomic operations on system variables, so that there’s no interference between tasks (creating, deleting, changing variables) during a read or write of a variable, which often involves GSTrans (and EvaluateExpression – missed that).

You may have missed it, but the code fragments are copied for the length of the fragment.

No, I saw that, but what I had missed was that code variables return a pointer to and length of their output string, but there’s no guarantee that the same string with the same length will be returned. That rather throws a spanner in the works.

Consider a Read$Counter variable that returns the number of times it’s been read. Perform the standard

read for length, allocate buffer, read to fill buffer

Except it doesn’t do that, at least with

echo \<Read$Counter\>

Show Read$Counter

I’m not sure what you mean by having a flag for the variable, though

The flag I suggested was to block the (visible) deletion of the variable that has just been found in a search. IDK what happens at the moment if you try to continue a search after you’ve deleted the most recently found match?

The plan was to implement GSInit/GSRead using GSTrans. Get the whole variable value into local storage, read it out a character at a time, free at the end or at the next GSInit (GSTrans no longer using GSInit/GSRead, that should be safe, I hope.)

Thanks for the hint about strange deletion of code variables. I wonder why it does that!

My quick and dirty test of a code variable:

   10 DIM C% 1000

   20 PRINT ~C%

   30 FOR opt%=0 TO 1

   40   P%=C%

   50   [ OPT opt%*3

   60   MOV pc, r14 ; definitely never said mov pc, r15, honest!

   70   str lr, [sp, #-4]!

   80   ldr r0, number

   90   add r0, r0, #1

  100   str r0, number

  110   adr r1, string

  120   mov r2, #12

  130   swi &d8

  140   rsb r2, r2, #12

  150   ldr pc, [sp], #4

  160   .number dcd 0

  170   .string

  180   dcd &2a2a2a2a

  190   dcd 0

  200   dcd 0

  210   ]

  220 NEXT

  230 SYS"OS_SetVarVal", “Read$Counter”, C%, P%-C%, 0, 16 TO ,,, N%, T%

  240 PRINT ~N%, ~T%

  245 DIM M% 1000

  250 PROCread

  260 PROCread

  270 END

  280 DEF PROCread

  290 SYS"XOS_ReadVarVal", “Read$Counter”, 0, -1, 0, 0 TO ,,L%

  300 L%=NOT L%

  310 PRINT L%

  320 SYS"OS_ReadVarVal", “Read$Counter”, M%, L%, 0, 0 TO ,,L%

  330 L%=NOT L%

  340 M%?L%=13

  350 PRINT L%, $M%

  360 ENDPROC

What I need in a multi-threaded preemptive environment are

RISC OS is really bad in this respect. Have you considered what happens if you interrupt an app when it’s spewing bytes to the VDU driver?
Or two pre-empted Wimp apps that are loading their templates at the same time? (not impossible, consider apps run at startup)
Or, well, anything using fonts.

This is why I think that it’s best to treat other cores using a Tube-like mechanism rather than trying to make RISC OS multicore capable. RISC OS would need to understand processes, with individual system context for each. And somehow compatible with all that’s come before… (and let’s not talk about SendMessage_Recorded)

Line 60 ? Also you should preserve R0 over the read.

I love line 60… 8~D

I thought I remembered this:

Note

MOV Rd,Rm syntax is allowed with Rd or Rn = pc, but not both. All other cases are
deprecated.

It’s not just nonsense, it’s actually illegal, and any competent assembler will barf on it.

(No, I don’t remember everything I ever wrote…)

(I’ve also just noticed an error on the very next line, at the top of the following page in the 2010 edition that I have a copy of…

If you use pc as Rd, the value used is the address of the instruction plus 8.

Should be Rn there…)

Oh, bother (line 60). Fortunately, I never tried to write to the variable! It was obviously supposed to be `mov pc, lr`. I did say it was quick and dirty!

I don’t have to preserve r0, it’s where you return the pointer to the value. PRM 1-320.

Get rid of line 330, as well, and put some whitespace in 350. Then you do get what I anticipated, which you don’t with Show or echoing the variable. Strange.

RISC OS is really bad in this respect.

Yes.

Have you considered what happens if you interrupt an app when it’s spewing bytes to the VDU driver?

Yes. It’s just a shared resource that will be protected by locks. There could be one VDU driver per system, per core, or per app that needs it. I haven’t decided yet, but it’s a relatively simple variation. Many programs don’t need to have anything to do with the VDU drivers, I have another mechanism for transferring streams of bytes. https://github.com/Simon-Willcocks/RISC-OS-Kernel-in-C/wiki/PipeOp (I know, it could do with more detail.)

Or two pre-empted Wimp apps that are loading their templates at the same time?

Ye-es. It won’t happen. The Wimp will work cooperatively, as before, but allow threads in the apps to operate update window loops in the background. As soon as any window position altering Wimp call is made, their loops will come to an abrupt end. The visible rectangles will be cached in between times.

Or, well, anything using fonts.

Shared resource(s), again.

So, any takers?

Have you considered what happens if you interrupt an app when it’s spewing bytes to the VDU driver?

One of those old habits that you’d need to break, and it looks like Simon is breaking the habit.

Hopefully, Wimp tasks won’t see a difference unless they want to. A/V applications should be able to update video and, more importantly, audio in the background.

If I can get it working.

Oh, bother (line 60). Fortunately, I never tried to write to the variable! It was obviously supposed to be `mov pc, lr`.

You’re forgiven. I was probably responsible for the error in the Assembler Guide, although the copy I have is dated four years after I last worked on it, and probably ten years after I first wrote that line (if I did), so nobody had noticed it – or not mentioned it if they had – in all that time.

I don’t have to preserve r0, it’s where you return the pointer to the value. PRM 1-320.

Ha. Teach me to look at the online reference (now fixed) rather than the book.