On Thu, Feb 9, 2012 at 5:55 PM, Peter Stuge <[email protected]> wrote: > Andrew Parlane wrote: >> I'm trying to compile openocd for the zy1000, using the current HEAD: >> f86986a9ef9ca863bad3bec5ffbdea748d011b40 >> >> Unfortunately I'm getting an error during compilation. I should add >> that a few months ago I succeeded in building, and it is only since >> I pulled the last few months changes in that I came across the >> problem. > .. > >> The error is: >> >> cc1: warnings being treated as errors >> stm32_stlink.c: In function 'stm32_stlink_read_memory': >> stm32_stlink.c:648: warning: cast increases required alignment of target type >> stm32_stlink.c: In function 'stm32_stlink_write_memory': >> stm32_stlink.c:697: warning: cast increases required alignment of target type > .. >> ./configure --enable-ioutil --enable-zy1000 --host=nios2-linux-gnu > > The line currently looks like > > res = stlink_if->layout->api->read_mem32(stlink_if->fd, address, c, (uint32_t > *)dst); > > The offending commit is > > commit c2ab3b4d5efba0201d88d899dc0fd310574ef5b9 > Author: Mathias K <[email protected]> > Date: Thu Jan 12 21:07:57 2012 +0100 > > stlink: add none 32bit memory read/write functions > > This patch add none 32bit memory read/write functions. > > Change-Id: Ie3a761cf006249b30d0691d1ea167d69a012c36a > Signed-off-by: Mathias K <[email protected]> > Reviewed-on: http://openocd.zylin.com/367 > Tested-by: jenkins > Reviewed-by: Spencer Oliver <[email protected]> > > which added the cast, as part of also supporting accesses of sizes > other than 32 bits.
The cast was there before, just in another place. > Assuming that the code worked earlier, it seems that the input > parameter "buffer" will be correctly aligned anyway. But I guess it's > not guaranteed, or gcc would not complain. So if read_mem32() is to > be kept and used, then the calling code must also work with a buffer > which is guaranteed to be aligned properly. Possibly not so trivial > to fix. Several problems here. The root problem is the API design "bug" to make the host buffer type dependent on the target access size, when they are completely unrelated concepts. In stlink_layout.h: int (*read_mem8) (void *handle, uint32_t addr, uint16_t len, uint8_t *buffer); int (*read_mem32) (void *handle, uint32_t addr, uint16_t len, uint32_t *buffer); These should have the same signatures, and the buffer type should arguably be void*, which implies that buffer doesn't need to be aligned in any particular way. I've seen the same mixup of host/target requirements in other APIs in OpenOCD, always a source of confusion and accompanied by a bunch of unsafe casts. In this case, the actual implementations apparently memcpy the data to the buffer anyway, so it should be safe to simply change the type of read_memX and remove all the casts. The same applies to write_memX, btw. A related problem is that the compiler doesn't complain about these bad casts on x86. They are never OK in portable code and we should probably switch on a warning for those in the jenkins build. A less obvious problem is that stm32_stlink.c is even compiled when the stlink interface isn't configured. The ZY1000 in particular should have very little use for that target. /Andreas ------------------------------------------------------------------------------ Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d _______________________________________________ OpenOCD-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openocd-devel
