[Libguestfs] [PATCH v3 1/3] New API: internal_find_block

[Pino Toscano]

On Tuesday, 20 September 2016 16:19:30 CEST Matteo Cafasso wrote:
> +  for (index = 0; index < count; index++) {
> +    fsattr = tsk_fs_file_attr_get_idx (fsfile, index);
> +
> +    if (fsattr != NULL && fsattr->flags & TSK_FS_ATTR_NONRES)
> +      tsk_fs_attr_walk (fsattr, flags, attrwalk_callback, (void *) &blkdata);

The return code of tsk_fs_attr_walk must be checked.

> +/* Attribute walk, searches the given block within the FS node attributes.
> + *
> + * Return TSK_WALK_CONT on success, TSK_WALK_ERROR on error.
> + */
> +static TSK_WALK_RET_ENUM
> +attrwalk_callback (TSK_FS_FILE *fsfile, TSK_OFF_T offset,
> +                   TSK_DADDR_T blkaddr, char *buf, size_t size,
> +                   TSK_FS_BLOCK_FLAG_ENUM flags, void *data)
> +{
> +  findblk_data *blkdata = (findblk_data *) data;
> +
> +  if (!(flags & TSK_FS_BLOCK_FLAG_SPARSE) && blkaddr == blkdata->block) {
> +    blkdata->found = true;

If we want to ignore sparse blocks, wouldn't it make sense to pass
TSK_FS_FILE_WALK_FLAG_NOSPARSE as additional flag to tsk_fs_attr_walk
above?

Also, my concerns about this that I replied in v2 still stand: is the
documentation obsolete, or does it document what is the expected
behaviour? In the former case, then it could be ok to partially
disregard what it said; in the latter, the code should follow what
it describes.

In any case, you should get in touch with the sleuthkit developers,
and get their feedback about that.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20160923/cdb0438f/attachment.sig>