Opened 7 years ago

Last modified 4 years ago

#55 assigned defect

Files without eas show an ea size of 128 bytes and without the /eas switch all files show ea size as 128

Reported by: Gregg Young Owned by: Valery V. Sedletski
Priority: minor Milestone: Future
Component: IFS Version:
Severity: low Keywords:
Cc:

Description

I see this in FM/2's details view.

Change History (10)

comment:1 Changed 7 years ago by Gregg Young

This code block appears several times in FillDirEntry0 and FillDirEntry1 in ifs\ifsfind.c. As you might guess EAMINSIZE = 128. Why is this needed?

                  if (!f32Parms.fEAS || !HAS_EAS( fEAS ))
                     /* HACK: what we need to return here
                        is the FEALIST size of the list
                        that would be produced by usGetEmptyEAs !
                        for the time being: just tell the user
                        to allocate some reasonably sized amount of memory
                     */   
                     pfFind->cbList = EAMINSIZE;
                     //pfFind->cbList = sizeof pfFind->cbList;
                  else
                     {
                     rc = usGetEASize(pVolInfo, pFindInfo->pInfo->rgClusters[0], NULL,
                        szLongName, &pfFind->cbList);
                     if (rc)
                        /* HACK: what we need to return here
                           is the FEALIST size of the list
                           that would be produced by usGetEmptyEAs !
                           for the time being: just tell the user
                           to allocate some reasonably sized amount of memory
                        */   
                        pfFind->cbList = EAMINSIZE;
                        //pfFind->cbList = 4;
                     rc = 0;
                     }

comment:2 Changed 7 years ago by Valery V. Sedletski

Please ask Lars Erdmann. He added a workaround for one problem (see ticket #40). He thinks that some minimal empty FEALIST structure is required to be returned even if a file hasn't EA's. I don't know if it's true, but he fixed a EA-related problem on his machine this way. This problem isn't reproduced on my machine, though.

comment:3 Changed 7 years ago by erdmann

Why is the EA size a problem ?

The real problem is this: even if a file has no EAs or if the /EAS switch is not set (which is equivalent to saying that a file should never return any EAs) the IFS has to return a length equal to a minium sized FEAList structure. This structure will return the requested EA names but without any EA content (as there is none) even if no EAs exist/should be returned. This structure will always be > 0 bytes. The intent is for a caller to be able to subsequently allocate a minimum sized buffer to receive that minimum FEAList. The perfect solution would be to dynamically compute that FEAList length (which can be done). It might be smaller than 128 Bytes but if many different EAs are requested it might even be > 128 Bytes.

In any case, expecting this length to be = 0 or 4 as in the old implementation (for no EAs) is the wrong expectation and no SW should expect this or rely on this.

comment:4 Changed 7 years ago by Gregg Young

Why is the EA size a problem ?

File managers report ea size in details view. It should always be correct. 128 implies the file has eas when it does not.

The problem is that while you need to pad usNeededLens and the checks to avoid overflowing the buffer, cblist should report the real value (zero in this case). FM/2 does this in its EA editor. It pads the buffer size (lens) with 24 bytes. I don't know if the person who wrote it knew that was what was needed or not. I changed the first EAMINSIZE in all occurrences of the code block above to zero and have had no issues using the driver. The rc != 0 case should probably fail outright not report an arbitrary value. I left EAMINSIZE in all the "lens" and "buffer checks" where they are needed.

In any case, expecting this length to be = 0 or 4 as in the old implementation (for no EAs) is the wrong expectation and no SW should expect this or rely on this.

The ea size is reported as zero for files without eas on HPFS, JFS, FAT and HPFS386 so FAT32 should also report zero. Thanks

comment:5 Changed 7 years ago by erdmann

Hallo Valery,

Gregg Young is correct. We should return 0 for all the places that he suggests.
That is, for all the relevant places we should return:
pfFind->cbList = 0; (and exactly 0 !)

instead of:
pfFind->cbList = EAMINSIZE;

comment:6 Changed 7 years ago by Valery V. Sedletski

2Lars: ok, I'll change EAMINSIZE to 0 in all such places. Thanks.

comment:7 Changed 7 years ago by erdmann

Sorry, but you should only change pfFind->cbList = 0; For the "usNeededLength" you should have stuck to EAMINSIZE;

comment:8 Changed 7 years ago by Valery V. Sedletski

Ah, ok. Returned back EAMINSIZE in all buffer size checks. Updated binaries are uploaded for r320.

PS: BTW, I also added a new feature: support for CDROM/DVD-ROM/CDRW/DVDRW media in FORMAT/CHKDSK/SYS. CD's with a FAT filesystem worked for a long time. I created a FAT16 filesystem with a 2048 byte per sector (with QSINIT on a ramdisk, then booted OS/2 and took an image) and burned it to a CDR disk. Fat32.ifs mounted it successfully and displayed a CD root directory. I was able to copy files from this FAT-formatted CDR. But since now, you can format a CDRW to FAT16/FAT32/exFAT filesystem, using a standard FORMAT tool. Also, CHKDSK can display FS info and fix FS erorrs. SYSINSTX command works too, but a CD won't boot, for some reason. (This is I added CD sector reading/writing routines. These routines were copied from my physical CDROM reading/writing support in VirtualBox?). So, this is just works like UDF-formatted CD's. You can copy in/out or delete files to such medium, just like any floppies or hard disks. Note that low level formatting of CDRW's is not yet supported. (As it is with low-level formatting of floppies). So, you should low-level format a CDRW with UDF first, like this:

format u: /fs:udf /f

Where /f (or /l) switch means low-level formatting. Then you should high-level format it to FAT16/FAT32/exFAT filesystem, like this:

format u: /fs:fat16

I tested all three file systems. All three is working with CDRW in OS/2 with fat32.ifs. On windows, FAT16 and FAT32-formatted CD's are successfully mounted and read, but not written. And free space == 0. So, it works read-only. With exFAT, such CD's are not even mounted under windows (both Win7 and WinXP).

Please test and have fun ;-)

comment:9 Changed 4 years ago by martini

Hi

Is there any more feedback on this ticket? Should we close it as "fixed"?

Regards

comment:10 Changed 4 years ago by martini

Owner: set to Valery V. Sedletski
Status: newassigned
Note: See TracTickets for help on using tickets.