On Jan 23, 2014, at 10:17 PM, Andreas Rohner wrote:
>
>> Do you free allocated memory in the case of error?
>
> Yes I immediately free it before I check the return value. Could you
> please read the code a bit more carefully, before you ask such questions?
+ freeblocks = (nilfs_get_blocks_per_segment(nilfs) * n)
+ - (nilfs_vector_get_size(vdescv)
+ + nilfs_vector_get_size(bdescv));
+
+ /* if there are less free blocks than the
+ * minimal threshold try to update suinfo
+ * instead of cleaning */
+ if (freeblocks < minblocks * n) {
+ ret = gettimeofday(&tv, NULL);
+ if (ret < 0)
+ goto out_lock;
+
+ supv = malloc(sizeof(struct nilfs_suinfo_update) * n);
^^^^^^ memory allocation
+ if (supv == NULL) {
+ ret = -1;
+ goto out_lock;
+ }
+
+ for (i = 0; i < n; ++i) {
+ supv[i].sup_segnum = segnums[i];
+ supv[i].sup_flags = 0;
+ nilfs_suinfo_update_set_lastmod(&supv[i]);
+ supv[i].sup_sui.sui_lastmod = tv.tv_sec;
+ }
+
+ ret = nilfs_set_suinfo(nilfs, supv, n);
+ free(supv);
^^^^ memory freeing
+ if (ret >= 0) {
+ /* success, tell caller
+ * to try another segment/s */
+ ret = -EGCTRYAGAIN;
+ goto out_lock;
+ }
+ }
+
Ok. But I can suggest to free memory in the place of error processing
(goto out_lock) and at the end of function for success. Otherwise, it
looks like hiding free() call. And it is very easy to forget about memory
freeing. It is good practice, I suppose, to group memory freeing operations.
Thanks,
Vyacheslav Dubeyko.
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
