All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Rajnoha <prajnoha@redhat.com>
To: lvm-devel@redhat.com
Subject: [PATCH 1/4] report: Print NULL strings as "" in _string_disp (instead of a SEGV).
Date: Tue, 16 Dec 2014 10:20:42 +0100	[thread overview]
Message-ID: <548FF96A.4020706@redhat.com> (raw)
In-Reply-To: <1418682760-16427-1-git-send-email-prockai@redhat.com>

On 12/15/2014 11:32 PM, Petr Rockai wrote:
> ---
>  lib/report/report.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/report/report.c b/lib/report/report.c
> index 5637d50..043de88 100644
> --- a/lib/report/report.c
> +++ b/lib/report/report.c
> @@ -179,6 +179,8 @@ static int _string_disp(struct dm_report *rh, struct dm_pool *mem __attribute__(
>  			struct dm_report_field *field,
>  			const void *data, void *private __attribute__((unused)))
>  {
> +	if (!*(void**) data)
> +		return _field_set_value(field, "", NULL);
>  	return dm_report_field_string(rh, field, (const char * const *) data);
>  }

...we could do this, but I think more appropriate here would be to
define field-specific reserved value to represent the "undefined"
value so we can match against this too when using selection
(-S|--select).

So, in lib/report/values.h, you can just define:

FIELD_RESERVED_VALUE(cache_policy, cache_policy_undefined, "", "", "undefined", ... add as many synonym to "undefined" here as you need ...)

With this, any "" or "undefined" (or any other synonym) can be used
in selection criteria to match against (the first synonym on the list
is the one printed - in this case it's blank value "").

But that would mean defining own display function for the
cache_policy, not using the generic _string_disp (but that's
not a problem, of course).

So the patch above is OK, but it depends how you need
the selection to behave when matching the cache_policy
field.

(I'm thinking whether the "" STR value should be a global
reserved value with that "undefined" and other synonyms,
but it's possible that wouldn't probably suit all STR fields...
I'd probably go with the field-specific reserved value for now.
We can make that generic anytime later...)

-- 
Peter



  parent reply	other threads:[~2014-12-16  9:20 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-15 22:32 [PATCH 1/4] report: Print NULL strings as "" in _string_disp (instead of a SEGV) Petr Rockai
2014-12-15 22:32 ` [PATCH 2/4] report: Add cache_policy and cache_settings (LV) segment fields Petr Rockai
2014-12-15 22:32 ` [PATCH 3/4] test: Add a test for reports of cache policy & settings Petr Rockai
2014-12-15 22:32 ` [PATCH 4/4] test: Check that cache settings properly survive LV reactivation Petr Rockai
2014-12-16  9:20 ` Peter Rajnoha [this message]
2014-12-16  9:56   ` [PATCH 1/4] report: Print NULL strings as "" in _string_disp (instead of a SEGV) Peter Rajnoha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=548FF96A.4020706@redhat.com \
    --to=prajnoha@redhat.com \
    --cc=lvm-devel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.