Re: [hatari-devel] Error when saving files

[Uwe Seimet <Uwe.Seimet@xxxxxxxxx>]

Hi Eero,

Yes, it's fixed, thank you.

Best regards

Uwe

> Hi Uwe,
> 
> It does not seem to be related to DTA population on Fsfirst/next().
> 
> I guess program reads Fattrib() before re-creating a file, and then 
> afterwards writes Fattrib() with the earlier queried attrib bitmask, 
> which was wrong.
> 
> I think it's fixed now.  Could you verify?
> 
> 
> 	- Eero
> 
> PS. Thanks again for reporting this, and sorry for committing such bugs!
> 
> On 8.12.2022 0.07, Uwe Seimet wrote:
> > Please see the attachment.
> >>
> >> Yes, please.  I need to understand *why* it breaks things to fix it
> >> properly.
> >>
> >> 	- Eero
> >>
> >> On 7.12.2022 23.55, Uwe Seimet wrote:
> >>> Hi,
> >>>
> >>> The patch fixes the issue. Do you still need the full trace based on the
> >>> non-patched sources?
> >>>
> >>> Best regards
> >>>
> >>> Uwe
> >>>
> >>>> Hi,
> >>>>
> >>>> On 7.12.2022 15.55, Uwe Seimet wrote:
> >>>>>> Are you sure this is (still) Hatari issue?
> >>>>>
> >>>>> Yes, I am sure because nothing else except Hatari would touch this file,
> >>>>> and . At
> >>>>> the beginning of the log these are the file permissions:
> >>>>>
> >>>>> -rw-rw-rw- 1 us users 4364 Dez  7 14:50 EDISON.RSM
> >>>>>
> >>>>> At the end of the log they are:
> >>>>>
> >>>>> -r--r--r-- 1 us users 4364 Dez  7 14:51 EDISON.RSM
> >>>>
> >>>> Ok, in that case I need more info, at least full "--trace gemdos" trace.
> >>>>
> >>>>
> >>>>> The commit causing this issue is 8fe983f2. Before this commit everything is
> >>>>> fine, as it always was.
> >>>>
> >>>> Does reverting just the GemDOS_ConvertAttribute() part (attached) of the
> >>>> commit, fix the issue?
> >>>>
> >>>>
> >>>> 	- Eero
> >>>
> >>>> --- a/src/gemdos.c
> >>>> +++ b/src/gemdos.c
> >>>> @@ -273,7 +273,7 @@ static bool GemDOS_SetFileInformation(int Handle, DATETIME *DateTime)
> >>>>    /**
> >>>>     * Convert from FindFirstFile/FindNextFile attribute to GemDOS format
> >>>>     */
> >>>> -static uint8_t GemDOS_ConvertAttribute(mode_t mode)
> >>>> +static uint8_t GemDOS_ConvertAttribute(mode_t mode, const char *path)
> >>>>    {
> >>>>    	uint8_t Attrib = 0;
> >>>>    
> >>>> @@ -282,7 +282,7 @@ static uint8_t GemDOS_ConvertAttribute(mode_t mode)
> >>>>    		Attrib |= GEMDOS_FILE_ATTRIB_SUBDIRECTORY;
> >>>>    
> >>>>    	/* Read-only attribute */
> >>>> -	if (!(mode & S_IWUSR))
> >>>> +	if (!(mode & S_IWUSR) || !access(path, W_OK))
> >>>>    		Attrib |= GEMDOS_FILE_ATTRIB_READONLY;
> >>>>    
> >>>>    	/* TODO, Other attributes:
> >>>> @@ -326,7 +326,7 @@ static dta_ret_t PopulateDTA(char *path, struct dirent *file, DTA *pDTA, uint32_
> >>>>    		return DTA_ERR;   /* no DTA pointer set */
> >>>>    
> >>>>    	/* Check file attributes (check is done according to the Profibuch) */
> >>>> -	nFileAttr = GemDOS_ConvertAttribute(filestat.st_mode);
> >>>> +	nFileAttr = GemDOS_ConvertAttribute(filestat.st_mode, tempstr);
> >>>>    	nAttrMask = nAttrSFirst|GEMDOS_FILE_ATTRIB_WRITECLOSE|GEMDOS_FILE_ATTRIB_READONLY;
> >>>>    	if (nFileAttr != 0 && !(nAttrMask & nFileAttr))
> >>>>    		return DTA_SKIP;
> >>>> @@ -2616,7 +2615,7 @@ static bool GemDOS_Fattrib(uint32_t Params)
> >>>>    	if (nRwFlag == 0)
> >>>>    	{
> >>>>    		/* Read attributes */
> >>>> -		Regs[REG_D0] = GemDOS_ConvertAttribute(FileStat.st_mode);
> >>>> +		Regs[REG_D0] = GemDOS_ConvertAttribute(FileStat.st_mode, sActualFileName);
> >>>>    		return true;
> >>>>    	}
> >>>>    
> >>>
> >>>
> >>>
> >>
> >>
> 
>