Re: [Xen-devel] [PATCH 1/3] libxlu_cfg: reject unknown characters following '\'

2016-02-17 Thread Jim Fehlig 
On 02/17/2016 03:11 AM, Ian Campbell wrote:
> On Wed, 2016-02-17 at 10:05 +, Ian Campbell wrote:
>> On Tue, 2016-02-16 at 20:54 -0700, Jim Fehlig wrote:
>>> When dequoting config strings in xlu__cfgl_dequote(), unknown
>>> characters following a '\', and the '\' itself, are discarded.
>>> E.g. a disk configuration string containing
>>>
>>>   rbd:pool/image:mon_host=192.168.0.100\:6789
>>>
>>> would be dequoted as
>>>
>>>   rbd:pool/image:mon_host=192.168.0.1006789
>>>
>>> Instead of discarding the '\' and unknown character, reject the
>>> string and set error to EINVAL.
>> Missing your S-o-b.
>>
>> Other than that:
>>
>>> +xlu__cfgl_lexicalerror(ctx, "invalid character after
>>> backlash "
>>> +   "in quoted string");
>> Please try where possible not to split string constants (so log messages
>> can more easily be grepped for).
> I see now that this parsing code is pretty liberally ignoring this advice
> already. So apart from the missing S-o-b this patch is

Your suggestion is a good one and is common throughout much of libxl, so I
pulled back the error string indentation in V2.

Regards,
Jim


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/3] libxlu_cfg: reject unknown characters following '\'

2016-02-17 Thread Ian Campbell 
On Wed, 2016-02-17 at 10:05 +, Ian Campbell wrote:
> On Tue, 2016-02-16 at 20:54 -0700, Jim Fehlig wrote:
> > When dequoting config strings in xlu__cfgl_dequote(), unknown
> > characters following a '\', and the '\' itself, are discarded.
> > E.g. a disk configuration string containing
> > 
> >   rbd:pool/image:mon_host=192.168.0.100\:6789
> > 
> > would be dequoted as
> > 
> >   rbd:pool/image:mon_host=192.168.0.1006789
> > 
> > Instead of discarding the '\' and unknown character, reject the
> > string and set error to EINVAL.
> 
> Missing your S-o-b.
> 
> Other than that:
> 
> > +xlu__cfgl_lexicalerror(ctx, "invalid character after
> > backlash "
> > +   "in quoted string");
> 
> Please try where possible not to split string constants (so log messages
> can more easily be grepped for).

I see now that this parsing code is pretty liberally ignoring this advice
already. So apart from the missing S-o-b this patch is

Acked-by: Ian Campbell 

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/3] libxlu_cfg: reject unknown characters following '\'

2016-02-17 Thread Ian Campbell 
On Tue, 2016-02-16 at 20:54 -0700, Jim Fehlig wrote:
> When dequoting config strings in xlu__cfgl_dequote(), unknown
> characters following a '\', and the '\' itself, are discarded.
> E.g. a disk configuration string containing
> 
>   rbd:pool/image:mon_host=192.168.0.100\:6789
> 
> would be dequoted as
> 
>   rbd:pool/image:mon_host=192.168.0.1006789
> 
> Instead of discarding the '\' and unknown character, reject the
> string and set error to EINVAL.

Missing your S-o-b.

Other than that:

> +xlu__cfgl_lexicalerror(ctx, "invalid character after 
> backlash "
> +   "in quoted string");

Please try where possible not to split string constants (so log messages
can more easily be grepped for). If that would result in more than 80
characters then it is acceptable to pull the string in more than you would
normally, e.g if this is (as I expect it will be) too long:

                xlu__cfgl_lexicalerror(ctx,
   "invalid character after backlash in 
quoted string");

then it's ok to do:
                xlu__cfgl_lexicalerror(ctx,
"invalid
character after backlash in quoted string");

(I usually try and keep it to a multiple of 4 spaces indent, FWIW) or for 
really long strings even:

                xlu__cfgl_lexicalerror(ctx,
 "invalid character after backlash in quoted string blah blah blah");

is preferable to breaking the string in half.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/3] libxlu_cfg: reject unknown characters following '\'

2016-02-16 Thread Jim Fehlig 
When dequoting config strings in xlu__cfgl_dequote(), unknown
characters following a '\', and the '\' itself, are discarded.
E.g. a disk configuration string containing

  rbd:pool/image:mon_host=192.168.0.100\:6789

would be dequoted as

  rbd:pool/image:mon_host=192.168.0.1006789

Instead of discarding the '\' and unknown character, reject the
string and set error to EINVAL.
---
 tools/libxl/libxlu_cfg.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c
index 1d70909..f8e0bc7 100644
--- a/tools/libxl/libxlu_cfg.c
+++ b/tools/libxl/libxlu_cfg.c
@@ -533,6 +533,11 @@ char *xlu__cfgl_dequote(CfgParseContext *ctx, const char 
*src) {
 NUMERIC_CHAR(2,2,16,"hex");
 } else if (nc>='0' && nc<='7') {
 NUMERIC_CHAR(1,3,10,"octal");
+} else {
+xlu__cfgl_lexicalerror(ctx, "invalid character after backlash "
+   "in quoted string");
+ctx->err= EINVAL;
+goto x;
 }
 assert(p <= src+len-1);
 } else {
-- 
1.8.0.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel