This is the mail archive of the
ecos-devel@sources.redhat.com
mailing list for the eCos project.
AW: AW: contributing a failsafe update meachanism for FIS from within ecos applications
- From: "Neundorf, Alexander" <Alexander dot Neundorf at jenoptik dot com>
- To: "Andrew Lunn" <andrew at lunn dot ch>
- Cc: "Slawek" <sgp at telsatgp dot com dot pl>, <ecos-devel at sources dot redhat dot com>
- Date: Mon, 25 Oct 2004 11:07:42 +0200
- Subject: AW: AW: contributing a failsafe update meachanism for FIS from within ecos applications
Hi,
> Von: Andrew Lunn [mailto:andrew@lunn.ch]
...
> > 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:
> >
...
> 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.
And that's why I changed from
unsigned short valid_flag;
to
unsigned char valid_flag1;
unsigned char valid_flag2;
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 fvi1;
memcpy(&fvi0, img0->name+sizeof(CYG_REDBOOT_VALID_MAGIC), sizeof(CYG_REDBOOT_VALID_MAGIC));
memcpy(&fvi1, img1->name+sizeof(CYG_REDBOOT_VALID_MAGIC), sizeof(CYG_REDBOOT_VALID_MAGIC));
vs.
struct fis_valid_info* fvi0 =(struct fis_valid_info*)buf0;
struct fis_valid_info* fvi1 =(struct fis_valid_info*)buf1;
which is IMO easier readable and has less potential for bugs, while also having no alignment problems (with my change to two characters).
> 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.
This has to be kept in mind anyway when changing the binary format of fis_image_desc. Changing this format means caring about compatibility with previous redboot versions, so it doesn't happen easily (as we are doing right now ;-)
Maybe something with a union would make it more explicit ?
union
{
struct fis_image_desc img;
struct fis_valid_info valid;
} give_me_a_name;
> > 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...
Yes, sure :-)
(I may have mentioned it already: the current implementation is not intended to go straight into ecos cvs, the conversion to an ecos fs will happen around end of november)
Issues I know of: eocs package and cdl-optins, namespace to ecos fs conversion incl. data structures, adding special function for update.
Bye
Alex