On Thu, Oct 9, 2014 at 6:23 AM, <scan-admin@xxxxxxxxxxxx> wrote:
>
> Hi,
>
> Please find the latest report on new defect(s) introduced to ceph found with Coverity Scan.
>
> 3 new defect(s) introduced to ceph found with Coverity Scan.
> 4 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 3 of 3 defect(s)
>
>
> ** CID 1244227: Dereference after null check (FORWARD_NULL)
> /mds/Server.cc: 7011 in Server::do_rename_rollback(ceph::buffer::list &, mds_rank_t, std::tr1::shared_ptr<MDRequestImpl> &, bool)()
> /mds/Server.cc: 7130 in Server::do_rename_rollback(ceph::buffer::list &, mds_rank_t, std::tr1::shared_ptr<MDRequestImpl> &, bool)()
These lines are
if (in && in->is_dir() && srcdn->authority().first != whoami) ...
and
} else if (force_journal_src || (in && in->is_dir() &&
srcdn->authority().first == whoami)) { ...
Coverity is complaining about the srcdn dereference, and I've dug into
it a bit but I think this might actually be an issue. Well, more
accurately, I think maybe if srcdn is NULL we've failed somehow and
should have given up, but the code looks to be not failing on purpose,
so I'm missing something. We should dig into this and either fix or
promote the
if (in && in->is_dir())
assert(srcdn && destdn);
which we have nested inside of a check for rollback.orig_src.ino (ie,
we were auth/primary for the srcdn at rename time).
The other two I've sent in an (untested) PR for:
https://github.com/ceph/ceph/pull/2677
>
> ** CID 1244228: Uninitialized scalar field (UNINIT_CTOR)
> /mds/MDSAuthCaps.h: 29 in MDSCapSpec::MDSCapSpec()()
The "read" cap bool is indeed uninitialized; easy enough to
default-fill it to false.
> ** CID 1244229: Uninitialized scalar field (UNINIT_CTOR)
> /messages/MDirUpdate.h: 55 in MDirUpdate::MDirUpdate(mds_rank_t, dirfrag_t, int, std::set<int, std::less<int>, std::allocator<int>> &, filepath &, bool)()
"discover" is indeed uninitialized by default (although it looks like
the only caller overrides that default). The PR sets it to 0, which
appears to be the correct default from my reading of the code.
-Greg
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
