[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: New version of function mkdirhier()



On Fri, 2012-01-13 at 05:02 +0100, Guillem Jover wrote:
> On Thu, 2012-01-12 at 18:07:46 +0100, Svante Signell wrote:
> > I'm currently creating a patch to make libtar build and stumbled on the
> > function mkdirhier() in util.c. Since that function use not recommended
> > and potentially dangerous functions like strlcpy and strsep I rewrote
> > and generalized it using strchr and strncat.
> 
> How are those functions dangerous? And not recommended by whom? They
> can cause portability issues but in this case the source provides
> replacements when the system lacks them, so that's a non-issue. In
> any case I have strong doubts upstream would accept such change.

FYI: There is no upstream, latest release was in 2003. Still some
software is dependent on libtar, that's the reason for porting.

>From the man page of strsep:
BUGS
Be cautious when using this function.  If you do use it, note that:
  * This function modifies its first argument.
  * This function cannot be used on constant strings.
  * The identity of the delimiting character is lost.
(and you need the extra library libbsd)

strtok is recommended, but additional problems here!
* The  strtok()  function  uses a static buffer while parsing, so it's
not thread safe.  Use strtok_r() if this matters to you.

> Using strncat OTOH tends to confuse people, the len param is a
> limiter on the src not the dest string. I see you made that mistake
> on the code. This implies that when the string has the same len as
> passed, the call is equivalent to its strcat counterpart, 

and what is wrong with using strncat compared to strcat?

> like in
> “strncat(foo, "/", 1)”, the other cases would need their len argument
> recomputed to pass the remaining dest space, which will end up with an
> even larger and more complex implementation.

see above!

> So I'd say, just switch the code to dynamically allocate the strings,
> and let it use strlcat/strlcpy/strsep, etc.

The problem with dynamic string allocation is that you cannot free
strings being modified by the functions, like strsep. Maybe you can find
a smart solution, I haven't. Are you ever happy with code written by
somebody else than yourself?
BTW: what happened with the psmisc patch you promised a long time ago?



Reply to: