From mboxrd@z Thu Jan 1 00:00:00 1970 From: mathieu.desnoyers@efficios.com (Mathieu Desnoyers) Date: Thu, 14 Nov 2013 16:14:20 +0000 (UTC) Subject: [lttng-dev] [lttng-tools PATCH 3/4] Correct the behavior of the utils_expand_path function In-Reply-To: References: <1384320878-28808-1-git-send-email-raphael.beamonte@gmail.com> <20131112195134.GB12183@thessa> <1384320878-28808-4-git-send-email-raphael.beamonte@gmail.com> <722861522.67118.1384438214771.JavaMail.zimbra@efficios.com> Message-ID: <1517784416.67313.1384445660213.JavaMail.zimbra@efficios.com> ----- Original Message ----- > From: "Rapha?l Beamonte" > To: "Mathieu Desnoyers" > Cc: "David Goulet" , lttng-dev at lists.lttng.org > Sent: Thursday, November 14, 2013 11:00:12 AM > Subject: Re: [lttng-dev] [lttng-tools PATCH 3/4] Correct the behavior of the utils_expand_path function > > 2013/11/14 Mathieu Desnoyers : > > Comments should start with a: > > > > /* > > * Then start of comment... > > Right. Should I submit a new patch for that ? A patch that apply on top of lttng-tools master. > > > There is no point in ending a "" with \0, it implicitly ends with a \0 > > already. > > > >> + strncmp(end_path, ".\0", 2) == 0) { > > > > Same here. > > Was done like that for clarity, as "implicit" is not "clear". "" with \0 is just sign that someone does not understand what he is doing. It's not clearer. > Would it > be preferable to hide it but still compare the first 2 chars of a 1 > char string ? (respectively 3 chars of a 2 chars string) I think you want to keep the comparison with 3 chars, but you might want to test it further to confirm. The comparison between the implied \0 and a character in the other string should do what you are looking for. > > > Not necessarily. What happens in the unlikely case someone passes a path > > with simply "/" ? > > Then we're not in this arm of the "if". If the path starts by (or is) > '/', it is considered as a "absolute path", and we're just in the arm > of the "if" that starts with the comment "/* If given path is already > absolute */" > > > So let's say we have: > > > > "../blah/../blah" > > > > so only "../" passed to realpath, or the entire "../blah/../blah" ? (let's > > suppose blah/ exists), right ? > > Perhaps "blah/" exists, perhaps "blah/" doesn't exist. In both cases, > it should not matter for our function. As we don't have any easy way > to know that directly, we are not considering it for the realpath > call, but only the "relative paths" that we can evaluate ("./" or > "../"). > You can look at the loop that starts with the comment "/* Split part > that will be resolved by realpath", and see that in this case, only > "../" will be passed to realpath. > > > In the "../blah/../blah" case, here the other ../ would be dealt with by > > utils_resolve_relative, is that it ? > > Exactly, as we can't pass it to realpath in case the directory does not > exist. OK. I look forward to seeing your reply for the other patch. Thanks, Mathieu > > Rapha?l > -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com