TFTP driver changes

[Eric Norum]

Thanks for the code walk-through.  I've tried to fix the problems you
found.

Till Straumann wrote:

> 
> I could discover memory leaks and other related problems at least under
> two conditions:
> 
> 1)  If an attempt to open("/TFTP/somedir/") fails for some reason after
>      the path evaluation succeeds (libc/open.c), the node 'loc' and hence
>      the pathname copy will never be released --> memleak.
>
> 2)  If an attempt to open("/TFTP/somedir/") _does_ succeed (e.g. because
>      a file (or directory) "somedir" exists on the host), 'loc' is copied to the
>      filesystem_location_info_t contained in the rtems_libio_t structure
>      and released, i.e. the string copy is free()ed leaving a dangling pointer
>      in the libio structure (maybe that's not a big drama - probably it's
>      never accessed anyway).

I think I've fixed both these problems in the following fashion.  An
attempt to open("/TFTP/somedir/",..) will fail because
rtems_tftp_eval_path() now rejects attempts to open() paths which end in
a / character.  As well, I set the node_access pointer to NULL after I
free the memory in rtems_tftp_free_node_info().

> 
> IMO, 1) is a bug in rtems proper. At least if the open fails after a successful
> path lookup, rtems_filesystem_freenode() should be called.

Sounds reasonable.  Not a problem for the TFTP driver though since open
is called after a successful path lookup only if the path is truly a
file, in which case no memory has been allocated.

> 
> For 2) it could be argued that node_access in the rtems_libio_t must not
> be accessed (because it was released at the end of 'open()'). However,
> IMO it would be cleaner to _not_ call ..._freenode() if open succeeds
> but to call it from close().

What's the opinion of the IMFS maintainers on these two suggestions?

> 
> Then there is another tiny problem with the TFTPfs modification:
> 
> rtems_tftp_eval_path() should append the pathname to pathloc->node_access
> if the latter is non_NULL because the lookup could also be relative to a CWD on
> TFTPfs...

I'm not sure that this is even possible.  The path handed to
rtems_tftp_eval_path has the "/TFTP/" stripped off the front so I don't
see how it would be possible for the routine to tell whether it's
working with a relative or absolute path.

> 
> Anyway - I would like a real RTEMS filesystem expert to explain the exact
> paradigms of using the node_access field.

Me too!

-- 
Eric Norum                                 eric.norum at usask.ca
Department of Electrical Engineering       Phone: (306) 966-5394
University of Saskatchewan                 FAX:   (306) 966-5407
Saskatoon, Canada.