kernel test robot <l...@intel.com> writes:

> Hi "Eric,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on bpf/master]
> [also build test WARNING on linus/master v5.9-rc1 next-20200817]
> [cannot apply to bpf-next/master linux/master]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    
> https://github.com/0day-ci/linux/commits/Eric-W-Biederman/exec-Move-unshare_files-to-fix-posix-file-locking-during-exec/20200818-061552
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git master
> config: i386-randconfig-m021-20200818 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <l...@intel.com>
>
> smatch warnings:
> kernel/bpf/task_iter.c:162 task_file_seq_get_next() warn: ignoring 
> unreachable code.

What is smatch warning about?

Perhaps I am blind but I don't see any unreachable code there.

Doh!  I see it.  That loop will never loop so curr_fd++ is unreachable.
Yes that should get fixed just so the code is readable.

I will change that.

Eric


>    128        
>    129        static struct file *
>    130        task_file_seq_get_next(struct bpf_iter_seq_task_file_info *info,
>    131                               struct task_struct **task)
>    132        {
>    133                struct pid_namespace *ns = info->common.ns;
>    134                u32 curr_tid = info->tid;
>    135                struct task_struct *curr_task;
>    136                unsigned int curr_fd = info->fd;
>    137        
>    138                /* If this function returns a non-NULL file object,
>    139                 * it held a reference to the task/file.
>    140                 * Otherwise, it does not hold any reference.
>    141                 */
>    142        again:
>    143                if (*task) {
>    144                        curr_task = *task;
>    145                        curr_fd = info->fd;
>    146                } else {
>    147                        curr_task = task_seq_get_next(ns, &curr_tid);
>    148                        if (!curr_task)
>    149                                return NULL;
>    150        
>    151                        /* set *task and info->tid */
>    152                        *task = curr_task;
>    153                        if (curr_tid == info->tid) {
>    154                                curr_fd = info->fd;
>    155                        } else {
>    156                                info->tid = curr_tid;
>    157                                curr_fd = 0;
>    158                        }
>    159                }
>    160        
>    161                rcu_read_lock();
>  > 162                for (;; curr_fd++) {
>    163                        struct file *f;
>    164        
>    165                        f = fnext_task(curr_task, &curr_fd);
>    166                        if (!f)
>    167                                break;
>    168        
>    169                        /* set info->fd */
>    170                        info->fd = curr_fd;
>    171                        get_file(f);
>    172                        rcu_read_unlock();
>    173                        return f;
>    174                }
>    175        
>    176                /* the current task is done, go to the next task */
>    177                rcu_read_unlock();
>    178                put_task_struct(curr_task);
>    179                *task = NULL;
>    180                info->fd = 0;
>    181                curr_tid = ++(info->tid);
>    182                goto again;
>    183        }
>    184        
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

Reply via email to