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

Re: New version of function mkdirhier()



On Fri, Jan 13, 2012 at 12:31:22PM +0100, Svante Signell wrote:
> > What ? Memory allocation has almost nothing to do with string operations
> > (except for strdup()). Here, strsep() modifies the string in place,
> > which means your buffer address doesn't change. You just have to save it
> > and free() it when you're done...
> 
> But we are talking strdup here. Regarding free(), see my comment to
> Samuel.

No, we're talking strsep(). See Samuel's reply "make a copy of the
pointer" which is exactly what I meant here by "You just have to save
it".

> I would prefer that Guillem answers himself, not you. Are you his
> advocate? BTW: How many patches to Debian packages do you create per
> day, I haven't seen any? Only Samuel, Pino and me have submitted recent
> bug reports. Yes, I know you are working on the gnumach/hurd
> functionality, and this is a good thing (tm). Not many people contribute
> in this area and all such contributions are of course more worth than
> porting buggy, non-maintained old code.

I value quality more than quantity. But I disagree about the "worth" of
such types of work. Porting is as valuable (I'd tend to say even more
considering the current state of the project) as system functionality.
But just as I force myself to write proper code before submitting it
for integration into kernel/system stuff, I expect porters to do just
the same with their contributions. And if others report problems, I
consider their comments and adjust the result accordingly. Again, I
expect others to do the same, which includes you. As for replying on
Guillem's behalf, you got it wrong. I was replying for myself since you
asked me to review your patch, which I did only after Guillem and Samuel
replies were posted.

> And the more you look at code the more you find
> out how large amounts of crappy code there are.

Completely off-topic. Your goal here was to fix a PATH_MAX related
issue, not fix the original code use of strlcpy/strlcat/whatever. The
original code being crappy (which isn't even the case here) has usually
little to do with that goal (except if it's so crappy it prevents you
from replacing PATH_MAX related code easily).

> > Now back to the technical stuff. I believe Guillem's solution is quite
> > appropriate. 
> 
> I did not see any proposed solution.

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

> The bad thing is that there are still packages depending on this code.
> It should go down the drain and depending packages convert to something
> else (or a new upstream pops up).

Sure, I never expressed anything against that.

> Might be so, in my opinion strncat is safer than strcat wrt buffer
> overflows, but I'm probably wrong.

When you're comfortable with string operations and won't e.g. forget
room for the null terminating char or waste time using calloc() where
malloc() would have been appropriate, you may have a different opinion.
String operations have been the subject of flame wars [1] for a long time
now, this isn't the place to start another.

-- 
Richard Braun

[1] http://sources.redhat.com/ml/libc-alpha/2000-08/msg00053.html


Reply to: