Re: [PATCH] vvfat: refresh writing long filename

[Michael Tokarev]

19.01.2025 04:07, BALATON Zoltan wrote:
> On Sun, 19 Jan 2025, Michael Tokarev wrote:
> > 18.01.2025 23:54, BALATON Zoltan wrote:
> > > > +typedef struct lfn_direntry_t {
> > > > +    uint8_t sequence;
> > > > +    uint8_t name01[10];
> > > > +    uint8_t attributes;
> > > > +    uint8_t direntry_type;
> > > > +    uint8_t sfn_checksum;
> > > > +    uint8_t name0e[12];
> > > > +    uint16_t begin;
> > > > +    uint8_t name1c[4];
> > > > +} QEMU_PACKED lfn_direntry_t;
> >
> > > > +static unsigned write_lfn_part(uint8_t *dest, unsigned dsize,
> > > > +                               const gunichar2 *lptr, const gunichar2 *lend)
> > > > +{
> > > > +    unsigned i;
> > > > +    for(i = 0; i < dsize / 2 && lptr + i < lend; ++i) {
> > > > +        dest[i / 2 + 0] = lptr[i] & 0xff;
> > > > +        dest[i / 2 + 1] = lptr[i] >> 8;
> > >
> > > Why not uint16_t and maybe cpu_to_le (or whatever that's called) if needed? May 
> > > be simpler than handling it byte by byte.
> >
> > The dest array is unaligned - this is, eg, name01 in the above struct.
> > Will it work to use entry->name01[i] = cpu_to_le16(lptr[i]) here,
> > provided lfn_direntry_t=>name is declared as uint16_t name[5] ?
>
> I think it should work, I don't see why it would not. The compiler should be able to figure out how to handle unaligned data where needed, you should 
> not need to do that by hand. Or I think you'd get a warning if it would not work.

Nope, it doesn't work.  Because when a (unaligned) pointer to this 2-byte
integer is passed to write_lfn_part(), this function doesn't know it is
unaligned.

The compiler already issues a warning at the point when we get address of
a field of a packed structure.  In this case, when calling write_lfn_part()
with lfn_direntry_t->name01 as an argument (and other names too).

And the code actually generates a trap on architectures where this matters, -
I tried a simple test program on sparc.

Thanks,

/mjt