[vdr] vdr 1.7.23: patch for handling symlinks in recordings directory as earlier

Klaus Schmidinger Klaus.Schmidinger at tvdr.de
Mon Feb 13 12:54:05 CET 2012


On 13.02.2012 12:27, Lucian Muresan wrote:
> On 13.02.2012 12:00, Klaus Schmidinger wrote:
>> On 13.02.2012 11:44, Lucian Muresan wrote:
>>> On 13.02.2012 10:43, Klaus Schmidinger wrote:
>>>>> On 01/27/2012 01:04 PM, Oliver Endriss wrote:
>>>>>> On Thursday 26 January 2012 11:07:18 Klaus Schmidinger wrote:
>>>>>>> On 25.01.2012 14:11, Oliver Endriss wrote:
>>>>>>>> On Wednesday 25 January 2012 10:29:16 Klaus Schmidinger wrote:
>>>>>>>>> On 17.01.2012 14:26, sundararaj reel wrote:
>>>
>>> [...]
>>>
>>>>> Well, with this patch, symbolic links are not displayed at all on my
>>>>> VDR machine, whereas with sundararaj reel's fix they are displayed
>>>>> correctly.
>>>>
>>>> So what you're saying it that this...
>>>>
>>>> ----------------------------------------------------------------------------------------------
>>>>
>>>>
>>>> --- a/recording.c
>>>> +++ b/recording.c
>>>> @@ -1120,9 +1120,13 @@ void cRecordings::ScanVideoDir(const char
>>>> *DirName, bool Foreground, int LinkLev
>>>> continue;
>>>> }
>>>> Link = 1;
>>>> +#if 0
>>>> + // do not resolve the symbolic links in paths to real path
>>>> + // thereby keeping all the recordings under one directory
>>>> buffer = ReadLink(buffer);
>>>> if (!*buffer)
>>>> continue;
>>>> +#endif
>>>> if (stat(buffer, &st) != 0)
>>>> continue;
>>>> }
>>>> ----------------------------------------------------------------------------------------------
>>>>
>>>>
>>>>
>>>> ...works, while this...
>>>>
>>>> ----------------------------------------------------------------------------------------------
>>>>
>>>>
>>>> --- recording.c 2012/01/25 09:32:39 2.45
>>>> +++ recording.c 2012/01/26 10:02:29
>>>> @@ -1120,11 +1120,6 @@
>>>> continue;
>>>> }
>>>> Link = 1;
>>>> - buffer = ReadLink(buffer);
>>>> - if (!*buffer)
>>>> - continue;
>>>> - if (stat(buffer, &st) != 0)
>>>> - continue;
>>>> }
>>>> if (S_ISDIR(st.st_mode)) {
>>>> if (endswith(buffer, deleted ? DELEXT : RECEXT)) {
>>>> ----------------------------------------------------------------------------------------------
>>>>
>>>>
>>>>
>>>> ...doesn't?
>>>>
>>>> I find that hard to believe, because the only difference here is that
>>>> the second version removes the stat() call, which is superfluous if
>>>> 'buffer' is no longer modified.
>>>
>>> Haven't really looked at the code, until now, and I also do not
>>> exactly know what the call to stat does and also didn't try to
>>> understand the whole picture now, but your patch does not simply
>>> remove just the call to stat, but also a *continue* statement from the
>>> *while* loop, this could have strong
>>> implications, so just please consider analyzing the issue with respect
>>> to that.
>>
>> The original code was
>>
>> ----------------------------------------------------------------------------------------------
>>
>> if (stat(buffer, &st) == 0) {
>> int Link = 0;
>> if (S_ISLNK(st.st_mode)) {
>> if (LinkLevel > MAX_LINK_LEVEL) {
>> isyslog("max link level exceeded - not scanning %s", *buffer);
>> continue;
>> }
>> Link = 1;
>> buffer = ReadLink(buffer);
>> if (!*buffer)
>> continue;
>> if (stat(buffer, &st) != 0)
>> continue;
>> }
>> ...
>> }
>> ----------------------------------------------------------------------------------------------
>>
>>
>> After Sundararaj's patch it looked like this (just leaving out the lines
>> that his '#if 0' disabled):
>>
>> ----------------------------------------------------------------------------------------------
>>
>> if (stat(buffer, &st) == 0) {
>> int Link = 0;
>> if (S_ISLNK(st.st_mode)) {
>> if (LinkLevel > MAX_LINK_LEVEL) {
>> isyslog("max link level exceeded - not scanning %s", *buffer);
>> continue;
>> }
>> if (stat(buffer, &st) != 0)
>> continue;
>> }
>> ...
>> }
>> ----------------------------------------------------------------------------------------------
>>
>>
>> Reducing this to the stat() calls results in
>>
>> ----------------------------------------------------------------------------------------------
>>
>> if (stat(buffer, &st) == 0) {
>> ...
>> if (stat(buffer, &st) != 0)
>> continue;
>> }
>> ...
>> }
>> ----------------------------------------------------------------------------------------------
>>
>>
>> As you can see, 'buffer' is no longer modified between the two calls, so
>> they
>> will both return the same value. The code sequence is only entered if
>> the first
>> stat() call returns 0, so the second call will also return 0, and thus the
>> 'continue' statement will never be executed.
>
> Now I looked a bit closer, and noticed that the "first" call to stat is actually not to *stat*, but to *lstat* in vanilla vdr-1.7.23, so if you remove the call to the second one, could it be that you're not really cutting redundancy, maybe it actually makes a difference?

Now you got me ;-)
I had switched back to 'stat()' instead of 'lstat()' and forgotten to reintroduce
the 'lstat()'. Therefore I saw two identical calls to stat().
With the first call being lstat(), the second call to stat() is of course necessary
for the following S_ISDIR() check.

Sorry for the confusion, and thanks for clarifying this.
So now this whole code sequence looks like this:

----------------------------------------------------------------------------------------------
            if (lstat(buffer, &st) == 0) {
               int Link = 0;
               if (S_ISLNK(st.st_mode)) {
                  if (LinkLevel > MAX_LINK_LEVEL) {
                     isyslog("max link level exceeded - not scanning %s", *buffer);
                     continue;
                     }
                  Link = 1;
                  if (stat(buffer, &st) != 0)
                     continue;
                  }
----------------------------------------------------------------------------------------------

Klaus



More information about the vdr mailing list