Here's some cleanups of the right fix in the Setup_arguments of the system.pp file for the Go32v2 in the trunk branch, see patch in attachement. Greetings, Marco.

The following section of this message contains a file attachment prepared for transmission using the Internet MIME message format. If you are using Pegasus Mail, or any other MIME-compliant system, you should be able to save it or view it from within your mailer. If you cannot, please ask your system administrator for assistance.

---- File information -----------
File: system.diff
Date: 30 Jun 2007, 16:57
Size: 1085 bytes.
Type: Unknown

Here's some cleanups of the right fix in the Setup_arguments of the system.pp file for the Go32v2 in the trunk branch, see patch in attachement.

The reason for the first change isn't clear to me
(what is being fixed or improved by this change) - I recommend to provide clarifications why a
change is needed when sending patches.

Tomas

Tomas Hajny wrote

The reason for the first change isn't clear to me (what is being fixed or improved by this change) - I recommend to provide clarifications why a change is needed when sending patches.

The problem fixed by r7012 is that the variable argv0len is incremented before the placement of the null character in the string cmdline so it is resulted in a out of range by one. The original fix solves it with an auxiliary variable that is not strictly necessary, when you can adjust the index. Marco.

PS: I sent the second part of the correction to the graph unit, I don't know if you are absent, in case please see at Graph unit under Go32v2 of the 12 Jun 2007.

First, I don't see why using the helper variable
should be worse than the newly proposed version.
Actually, I even believe that the optimizer might
be able to get rid of it if not needed (i.e. if
more efficient - on the other hand, using the
helper variable may be better because the
addition of length(doscmd) to argv0len is
performed only once rather than twice).

Second, as far as I can see, the proposed code
isn't semantically identical to the original one
(the terminating #0 is placed 1 byte earlier). So
unless there's a reason to change it (that's what
I asked about in the previous e-mail), I wouldn't
touch it. If you think that it needs to be
changed, please, provide clarification why it's
so and why the current version is wrong.

PS: I sent the second part of the correction to the graph unit, I don't know if you are absent, in case please see at Graph unit under Go32v2 of the 12 Jun 2007.

I was indeed on holiday at that time. However, I
can't test GO32v2 patches properly anyway at the
moment, so I decided not to apply them any longer
unless they are really obvious and I can reliably
foresee their effects. I'm sorry about that -
maybe someone else (like Pierre) can check and
apply them if appropriate.

Tomas

The purpose of this cleanup is for readability of source, and I think that a minor number of variable grows it.

I was not clear in previous e-mail, the focus of r7012 is just that the terminating #0 must be placed 1 byte earlier, by freezing the position with the variable cmdlen and skipping the inc(argv0len) which causes an off by one. My patch proposes a more simple way to obtain that.

I take effect of that, but in fact Pierre and others never answer to my segnalations, especially for those beyond the Go32v2 specific like the threads 'Nasm under FPC' and 'Question about assembler generators'.