Hello again :-)

I have reviewed the patch.

The old source code had 3 functions ft2232_init() that can call
ft2232_init_libftdi() or ft2232_init_ftd2xx() depending on a ft2232
ftdi communication library available. This is pretty simple and
coherent.

The new source code (after patching)  have 6 functions doing almost
the same: ft2232_init_ftd2xx(), ft2232_open_ftd2xx(),
ft2232_init_libftdi(), ft2232_open_libftdi(),  ft2232_init_sub(void),
ft2232_init() and the header update ;-)

As I understand the problem is with the ft2232_buffer=malloc() within
ft2232_init() that can fail after malloc causing function return
without ft2232_buffer=free().
The current behavior is as follows: Program returns error and finish
execution because it cannot run without interface. Operating system
makes memory page free on process termination.
The proposed solution is to free() allocated memory before
ft2232_init() returns.

I can understand that leaving allocated memory is bad. However there
is a simpler solution to the problem (patch attached) by only changing
few lines of existing code and leaving driver infrastructure/layout
common for all source code:

     ft2232_buffer_size = 0;
     ft2232_buffer = malloc(FT2232_BUFFER_SIZE); <-- this is the bad malloc

*     for (;;){

*        if (layout->init() != ERROR_OK) break; <-- we jump out on failure

          if (ft2232_device_is_highspeed())
          {
#ifndef BUILD_FT2232_HIGHSPEED
 #if BUILD_FT2232_FTD2XX == 1
               LOG_WARNING("High Speed device found - You need a newer
FTD2XX driver (version 2.04.16 or later)");
 #elif BUILD_FT2232_LIBFTDI == 1
               LOG_WARNING("High Speed device found - You need a newer
libftdi version (0.16 or later)");
 #endif
#endif
               /* make sure the legacy mode is disabled */
*               if (ft2232h_ft4232h_clk_divide_by_5(false) !=
ERROR_OK) break;
          }

          buf[0] = 0x85; /* Disconnect TDI/DO to TDO/DI for Loopback */
          if ((retval = ft2232_write(buf, 1, &bytes_written)) !=
ERROR_OK)
          {
               LOG_ERROR("couldn't write to FT2232 to disable
loopback");
*               break;
          }

#if BUILD_FT2232_FTD2XX == 1
          retval = ft2232_purge_ftd2xx();
#elif BUILD_FT2232_LIBFTDI == 1
          retval = ft2232_purge_libftdi();
#endif
*          if (retval != ERROR_OK) break;

*          return ERROR_OK; <-- do not run infinite loop, return ok if no error
     }

 *    free(ft2232_buffer); <-- and land here with freeing bad memory
 *    return ERROR_JTAG_INIT_FAILED;

In my opinion patch brings only unnecessary confusion and source code
complication, sorry, there is simpler solution. This program does not
need doubling function number and making code even more difficult to
read and understand as it is already enough complicated. Using more
function(void) and global structures only make things worst :-(

%./openocd -f interface/kt-link.cfg -f board/stm3210b_eval.cfg
Open On-Chip Debugger 0.5.0-dev-00895-g796086c-dirty (2011-06-01-20:16)
Licensed under GNU GPL v2
For bug reports, read
        http://openocd.berlios.de/doc/doxygen/bugs.html
Info : only one transport option; autoselect 'jtag'
1000 kHz
adapter_nsrst_delay: 100
jtag_ntrst_delay: 100
cortex_m3 reset_config sysresetreq
Info : max TCK change to: 30000 kHz
Info : clock speed 1000 kHz
Info : JTAG tap: stm32.cpu tap/device found: 0x3ba00477 (mfg: 0x23b,
part: 0xba00, ver: 0x3)
Info : JTAG tap: stm32.bs tap/device found: 0x16410041 (mfg: 0x020,
part: 0x6410, ver: 0x1)
Info : stm32.cpu: hardware has 6 breakpoints, 4 watchpoints

%./openocd -f interface/jtagkey2p.cfg -f board/stm3210b_eval.cfg
Open On-Chip Debugger 0.5.0-dev-00895-g796086c-dirty (2011-06-01-20:16)
Licensed under GNU GPL v2
For bug reports, read
        http://openocd.berlios.de/doc/doxygen/bugs.html
Info : only one transport option; autoselect 'jtag'
1000 kHz
1000 kHz
adapter_nsrst_delay: 100
jtag_ntrst_delay: 100
cortex_m3 reset_config sysresetreq
Info : max TCK change to: 30000 kHz
Info : clock speed 1000 kHz
Info : JTAG tap: stm32.cpu tap/device found: 0x3ba00477 (mfg: 0x23b,
part: 0xba00, ver: 0x3)
Info : JTAG tap: stm32.bs tap/device found: 0x16410041 (mfg: 0x020,
part: 0x6410, ver: 0x1)
Info : stm32.cpu: hardware has 6 breakpoints, 4 watchpoints
^C

It would be nice however to add Ctrl+C signal capture to gracefully
shutdown the program with proper deinitialization... maybe simple tcl
"quit" mapping would do the job :-)

Best regards :-)
Tomek

-- 
CeDeROM, SQ7MHZ, http://www.tomek.cedro.info

Attachment: 0001-Fixed-missing-free-after-malloc-in-ft2232_init.patch
Description: Binary data

_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to