This is the mail archive of the
mailing list for the eCos project.
Re: AW: contributing a failsafe update meachanism for FIS from within ecos applications
- From: Andrew Lunn <andrew at lunn dot ch>
- To: "Neundorf, Alexander" <Alexander dot Neundorf at jenoptik dot com>
- Cc: Slawek <sgp at telsatgp dot com dot pl>, Andrew Lunn <andrew at lunn dot ch>,ecos-devel at sources dot redhat dot com
- Date: Mon, 25 Oct 2004 10:48:01 +0200
- Subject: Re: AW: contributing a failsafe update meachanism for FIS from within ecos applications
- References: <5A8A17126B73AC4C83968F6C4505E3C5013729CC@JO-EX01.JENOPTIK.NET>
> Andrew: in your patch you removed the magic_name field from the valid_info struct. While this works, it requires quite a lot more typing:
> struct fis_image_desc* img0=(struct fis_image_desc*)buf0;
> struct fis_image_desc* img1=(struct fis_image_desc*)buf1;
> struct fis_valid_info* fvi0
> =(struct fis_valid_info*)img0->name+sizeof(CYG_REDBOOT_VALID_MAGIC);
> struct fis_valid_info* fvi1
> =(struct fis_valid_info*)img1->name+sizeof(CYG_REDBOOT_VALID_MAGIC);
This is wrong and dangerous and i do not do that in my code. Becasue
sizeof(CYG_REDBOOT_VALID_MAGIC) == 10, your fvi is not word
aligned. When you then access fvi0->valid_flag you do a none aligned
access which on ARM and probably other targets will throw an BUS
exception. That why my code does a memcpy() from the tail of the name
array into fvi.
> instead of:
> struct fis_valid_info* fvi0 =(struct fis_valid_info*)buf0;
> struct fis_valid_info* fvi1 =(struct fis_valid_info*)buf1;
What im trying to do is use fis_image_desc as much as possible becasue
thats what we have to be compatible with. By defining a new structure
there is the danger that fis_image_desc gets changed and
fis_valid_info does not.
> which IMHO makes it much easier to mix things up and introduce bugs,
> while it doesn't help against the padding/alignment problem (AFAIK).
It should help the padding/allignment problem becasue my structure is
aligned where as yours is not. I also memcpy it back and for which is
always safe to do. And the assert() i added should tell us if its gone
> This doesn't only apply to the redboot code, but even more to the
> fisfs implementation, there this has to be done more often.
I took a quick look at your fisfs code. We need to talk about that...