On 10/9/21 3:49 AM, jr wrote:
> hi,
> 
> William F Pokorny <ano### [at] anonymousorg> wrote:
>> On 10/6/21 5:01 AM, jr wrote:
>>> ...
>> This a bug is basically the same as the radiosity file append bug.
         x
> 
> a known issue, ok.  thanks.
> 

Hi jr,

No. Well, it perhaps now is. I wasn't smart enough to consider the 
radiosity sample file issue might indicate a general problem.

If you see other similar file/IO issues, please report them. Thanks to 
you both for reporting this one. Internally, it led to two issues in 
that the write and append modes were both not working.

---
I'd taken the radiosity samples file bug to be a one off problem. On 
seeing the same root cause for this fopen keyword issue, it's looking 
more like a systemic problem with user file IO in v3.8. :-(

While away from my computer the past few days, I found myself thinking 
about this issue. And prior to leaving, I'd looked back through the 
radiosity samples file fix to radiosity.cpp; This time looking at other 
IStream/OStream class usage. I saw additional concerning code.

I've not spent much time with POV-Ray's IO code. We have an IO class 
which doesn't end up as nullptr on a failed open, but rather an 
allocated class object with extra information - like the file name. Our 
our always allocated 'file handle, handler' can be in a fail state or 
not. That state could be due an attempt to open a file - or I guess any 
other cause thereafter.

I'm far from understanding all the particulars - including not 
understanding what changed in the v3.7 to v3.8 work that we have these 
issues popping up. There was obviously some IO class work - and quite a 
lot of code re-factoring around IO. I found a commit from 2018 making 
changes so the #read directive 'again worked.' Did #read not work in 
v3.7 and/or some other post v3.7 versions?

Further, I'm unsure why the FOPEN, Parse_Fopen() code is using a 
shared_ptr<IStream> when I see no reason for a shared pointer(1). The 
v3.7 code doesn't use a shared_ptr.

Plus, for the read code, there is extra code appending common suffixes 
(I guess potentially .csv here?) before failing. On unix/linux I'd like 
all the 'guess a file suffix' code gone! I believe, even on windows 
where suffixes are hidden by default, this 'guess a suffix' code likely 
causes user confusion as often as not(2).

Oh, I think I might now understand why using the 'if (*readPtr)' was 
causing  segfaults. It's that the particular 'open for read' code - 
looking for files while guessing suffixes - is 'sometimes' returning 
simply 'nullptr'.

So, sometimes, the 'if (fid != nullptr)' test is what you have to use 
while elsewhere you need 'if (*fid)'. This suggests, to be robust, we 
should be using 'if ((fid != nullptr) && (*fid))' in the latter case. 
Hmm... Or maybe in all cases as it should be safe and it's consistent!

I guess, I should spend time digging into POV-Ray's IO code though I'd 
rather be playing elsewhere... :-(

Bill P.

(1) - I had the thought, maybe with shared_ptr() what the coder was 
after was safe clean up when the last referencing object went out of 
scope / was deleted. But, if so, why wasn't the same thing done with the 
write and append OStream pointers... And if cleanup the reason, I think, 
these days, a unique_ptr is likely better.

(2) - "Cannot find file 'fopen_HAPPY_r.inc', even after trying to append 
file type extension." <-- Message used where it doesn't makes sense 
given the passed file name already has a valid extension and POV-Ray 
should not be second guessing the user.

Post a reply to this message