[Coco] DriveWire 4 seg fault under Linux Mint...

[Tormod Volden]

On Thu, Nov 13, 2014 at 10:23 PM, Aaron Wolfe <aawolfe at gmail.com> wrote:
> On Nov 13, 2014 4:01 PM, "Tormod Volden" <lists.tormod at gmail.com> wrote:
>>
>> On Tue, Nov 11, 2014 at 4:32 PM, Aaron Wolfe  wrote:
>> > Ugh.. now I remember why I didn't take action on some of those.. i just
>> > really dont know what is the right thing to do.
>> >
>> > Standard DW behavior for reading beyond the end of the file is to
> succeed
>> > and return Os.  That's been the behavior since long before I became
>> > involved.   There is a setting I added to make this fail but I can't
> make
>> > it the default without breaking compatibility with previous versions.
>>
>> Right, this is the "expand" parameter. To me it sounds most logical to
>> expand on write, but return an error on read. Because when reading,
>> the client should know the size of the file e.g. through a header,
>> however when building a file the client would need to expand it since
>> OP_NAMEDOBJ_CREATE makes an empty file.
>>
>> Does anyone know any examples of clients that need to read "phantom"
> sectors?
>
> It is common and normal to use a zero byte file as a "blank" disk.   We
> don't want read errors when for instance a user does a decb directory on
> such a disk.  By default DW returns 0s for any request to read beyond the
> end of a file, but only expands the file when a write is done (to fill
> zeros from current end of file to start of sector being written when
> needed).  This has been the behavior for many years before I started
> working on DW, I don't know all the ramifications of changing it and there
> may be more than are easily thought of.   I don't think this is safe to
> change.

OK, I think we can leave it like this, having the "expand" parameter
is good enough.

>
>>
>> However, I think I was mixing up this issue with another, which is
>> more clearly a bug: In some cases DriveWire4 does not return neither
>> data nor return code but just goes quiet. Very visible when using
>> Becker, since the client will hang forever (there is no timeout in the
>> HDB-DOS dwread routine for Becker, this is on my todo list).
>>
>> One example is when DriveWire4 is not able to read from a file. So if
>> a file has been mounted but disappears, DriveWire4 just fails without
>> returning read error $F4 to the client. The log shows:
>> ERROR  DWProtocolHandler   dwproto-0-11        IOError in proto op:
>> Could not read from ... because it is a not a file.
>>
>
> The DW spec uses a timeout of 200ms to indicate an operation has failed.
> In some cases, DW intentionally waits this long so that the timeout will be
> seen by the coco and it can behave accordingly.  Any implementation should
> respect this part of the protocol.  I don't know whether this particular
> instance is intentional or not,  but it sounds like something the client
> should be handling better in any case.

Ah, I hadn't realized the timeout was part of the signalling, I
thought it was just for robustness.

Then we certainly need to fix the Becker routines.

In this case though, returning a read error $F4 would be appropriate,
wouldn't it?

"If the Server was unable to acquire the requested sector due to an
error, then the Server shall return an appropriate read error code"

>
>> I would have made the server keep the "mounted" files open so that
>> they cannot disappear, or check for their existence on every read.
>>
>
> Holding the files open makes it impossible to do many handy things, such as
> using the toolshed tools to manipulate their contents.  Depending on the
> current settings, DW can sync any changes made by outside tools to any
> reads made by the coco.  Maybe it isn't handling the file going away well,
> maybe the timeout is by design, its been too long to recall.  If there is
> any issue it can be corrected without giving up the benefits of leaving
> images accessible to external tools.

Definitely prefer to not lock the file. It is very handy to be able to
manipulate the contents "live". I actually thought I had to remount
files that I had manipulated or rewritten externally, I am glad to
hear this is not necessary.

>
>> >
>> > That's not as bad as the "files must be sized to sanely match sector
> size"
>> > issue.. DW only reads and writes sectors so what should it so with a
> file
>> > that doesn't align to sectors?  Refuse to serve up the incomplete
> sector,
>> > or pad it with zeros, make it read only, ?  The current model is all
> about
>> > the sectors, no mechanisms for communicating to the coco whether we
> have a
>> > full sector or not so it can't make the call on thst side as needed.
>>
>> Padding any incomplete sector with zeroes is the most straight-forward
>> behaviour IMO. It won't change anything for existing full-sector use.
>> For writing there is no issue, because OP_WRITEX always needs a full
>> sector and the client can provide that.
>>
>> >
>> > I don't want to break compatibility or create a scenario where files are
>> > getting trashed without the user wanting them to be.  What's the correct
>> > approach on these?
>>
>> In what case could files be trashed?
>
> Any write to the last sector of a file is going to pad it out with
> something (determined by coco) since the only write operation is a full 256
> byte sector.  This means changing the size of the source file and probably
> making it not what a user wants.  There is no mechanism for writing less
> than a full sector anywhere in DW.
>
> I could make these sources read only to avoid issues if this is acceptable.

I would prefer the change of file size. If someone is using non-256
files it would be part of the game that they change size on rewrite.
If the file is going to be changed anyway, I don't see any practical
problem it being padded. Since we simply cannot write non-256 files
over DriveWire, the client shouldn't try to write to it if it doesn't
want the file size changed.

Regards,
Tormod



>
>>
>> Regards,
>> Tormod
>>
>> >  On Nov 11, 2014 10:15 AM, "Tormod Volden" wrote:
>> >
>> >> On Tue, Nov 11, 2014 at 3:57 PM, Aaron Wolfe wrote:
>> >> > I suspect this could be fixed if I knew about it..
>> >> >
>> >> > I will try to find some time to release a version that supports your
>> >> > project better.  Can you send me a list of all known issues relative
> to
>> >> the
>> >> > current release?   There are so many bugs and unfinished bits in that
>> >> test
>> >> > version from the xfer directory.   We need to start from a saner
> place.
>> >>
>> >> Great! I discussed this project with you in April and gave you a list
>> >> of issues, but it might have grown a bit since then.
>> >>
>> >> 1. Ability to mount "images" that are not multiple of 256 bytes
>> >> 2. create named object request doesn't work
>> >> 3. reading beyond end of file does not return error code (silently
> fails)
>> >> 4. version 4.3.3p crashes on Ubuntu 12.04 (4.3.4d works great)
>> >> 5. survive without something mounted in drive0-4
>> >> 6. Some clear GUI indication if the server is in Turbo mode (although
>> >> I know Turbo is not supported yet)
>> >>
>> >> Regards,
>> >> Tormod