* re: xfs: eliminate committed arg from xfs_bmap_finish
@ 2016-03-10 13:03 Dan Carpenter
2016-03-10 13:51 ` Eric Sandeen
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2016-03-10 13:03 UTC (permalink / raw)
To: sandeen; +Cc: xfs
Hello Eric Sandeen,
The patch f6106efae5f4: "xfs: eliminate committed arg from
xfs_bmap_finish" from Jan 11, 2016, leads to the following static
checker warning:
fs/xfs/xfs_bmap_util.c:132 xfs_bmap_finish()
error: XXX potentially using uninitialized 'committed'.
fs/xfs/xfs_bmap_util.c
98 int /* error */
99 xfs_bmap_finish(
100 struct xfs_trans **tp, /* transaction pointer addr */
101 struct xfs_bmap_free *flist, /* i/o: list extents to free */
102 struct xfs_inode *ip)
103 {
104 struct xfs_efd_log_item *efd; /* extent free data */
105 struct xfs_efi_log_item *efi; /* extent free intention */
106 int error; /* error return value */
107 int committed;/* xact committed or not */
108 struct xfs_bmap_free_item *free; /* free extent item */
109 struct xfs_bmap_free_item *next; /* next item on free list */
110
111 ASSERT((*tp)->t_flags & XFS_TRANS_PERM_LOG_RES);
112 if (flist->xbf_count == 0)
113 return 0;
114
115 efi = xfs_trans_get_efi(*tp, flist->xbf_count);
116 for (free = flist->xbf_first; free; free = free->xbfi_next)
117 xfs_trans_log_efi_extent(*tp, efi, free->xbfi_startblock,
118 free->xbfi_blockcount);
119
120 error = __xfs_trans_roll(tp, ip, &committed);
121 if (error) {
122 /*
123 * If the transaction was committed, drop the EFD reference
124 * since we're bailing out of here. The other reference is
125 * dropped when the EFI hits the AIL.
126 *
127 * If the transaction was not committed, the EFI is freed by the
128 * EFI item unlock handler on abort. Also, we have a new
129 * transaction so we should return committed=1 even though we're
130 * returning an error.
131 */
132 if (committed) {
"committed" is never initialized to zero. It's either 1 or
uninitialized.
133 xfs_efi_release(efi);
134 xfs_force_shutdown((*tp)->t_mountp,
135 (error == -EFSCORRUPTED) ?
136 SHUTDOWN_CORRUPT_INCORE :
137 SHUTDOWN_META_IO_ERROR);
138 }
regards,
dan carpenter
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: xfs: eliminate committed arg from xfs_bmap_finish
2016-03-10 13:03 xfs: eliminate committed arg from xfs_bmap_finish Dan Carpenter
@ 2016-03-10 13:51 ` Eric Sandeen
0 siblings, 0 replies; 2+ messages in thread
From: Eric Sandeen @ 2016-03-10 13:51 UTC (permalink / raw)
To: xfs
On 3/10/16 7:03 AM, Dan Carpenter wrote:
> Hello Eric Sandeen,
>
> The patch f6106efae5f4: "xfs: eliminate committed arg from
> xfs_bmap_finish" from Jan 11, 2016, leads to the following static
> checker warning:
>
> fs/xfs/xfs_bmap_util.c:132 xfs_bmap_finish()
> error: XXX potentially using uninitialized 'committed'.
You're right; thankfully it's "only" on an error path, but I'll
get a patch sent out to fix this up.
-Eric
> fs/xfs/xfs_bmap_util.c
> 98 int /* error */
> 99 xfs_bmap_finish(
> 100 struct xfs_trans **tp, /* transaction pointer addr */
> 101 struct xfs_bmap_free *flist, /* i/o: list extents to free */
> 102 struct xfs_inode *ip)
> 103 {
> 104 struct xfs_efd_log_item *efd; /* extent free data */
> 105 struct xfs_efi_log_item *efi; /* extent free intention */
> 106 int error; /* error return value */
> 107 int committed;/* xact committed or not */
> 108 struct xfs_bmap_free_item *free; /* free extent item */
> 109 struct xfs_bmap_free_item *next; /* next item on free list */
> 110
> 111 ASSERT((*tp)->t_flags & XFS_TRANS_PERM_LOG_RES);
> 112 if (flist->xbf_count == 0)
> 113 return 0;
> 114
> 115 efi = xfs_trans_get_efi(*tp, flist->xbf_count);
> 116 for (free = flist->xbf_first; free; free = free->xbfi_next)
> 117 xfs_trans_log_efi_extent(*tp, efi, free->xbfi_startblock,
> 118 free->xbfi_blockcount);
> 119
> 120 error = __xfs_trans_roll(tp, ip, &committed);
> 121 if (error) {
> 122 /*
> 123 * If the transaction was committed, drop the EFD reference
> 124 * since we're bailing out of here. The other reference is
> 125 * dropped when the EFI hits the AIL.
> 126 *
> 127 * If the transaction was not committed, the EFI is freed by the
> 128 * EFI item unlock handler on abort. Also, we have a new
> 129 * transaction so we should return committed=1 even though we're
> 130 * returning an error.
> 131 */
> 132 if (committed) {
>
> "committed" is never initialized to zero. It's either 1 or
> uninitialized.
>
> 133 xfs_efi_release(efi);
> 134 xfs_force_shutdown((*tp)->t_mountp,
> 135 (error == -EFSCORRUPTED) ?
> 136 SHUTDOWN_CORRUPT_INCORE :
> 137 SHUTDOWN_META_IO_ERROR);
> 138 }
>
> regards,
> dan carpenter
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-03-10 13:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-10 13:03 xfs: eliminate committed arg from xfs_bmap_finish Dan Carpenter
2016-03-10 13:51 ` Eric Sandeen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox