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

Lucian Muresan lucianm at users.sourceforge.net
Mon Feb 13 12:27:14 CET 2012


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?

Lucian



More information about the vdr mailing list