Re: Review Request 59108: PROTON-1466: proton-c mixing up links with names that are prefixes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59108/#review174548 --- Ship it! Ship It! - Andrew Stitcher On May 10, 2017, 8:34 p.m., Alan Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59108/ > --- > > (Updated May 10, 2017, 8:34 p.m.) > > > Review request for qpid, Andrew Stitcher and Gordon Sim. > > > Bugs: PROTON-1466 > https://issues.apache.org/jira/browse/PROTON-1466 > > > Repository: qpid-proton-git > > > Description > --- > > Creating links where one link name is a prefix of the other, e.g. "xx" and > "xxyy" sometimes caused the links to be confused. > > > Diffs > - > > proton-c/src/core/codec.c 67769ad8ac839242fcc08f008c60cf1b0e8a8bb8 > proton-c/src/core/transport.c 8777318db39e6304ee06c564579a751d0ca2307a > proton-c/src/core/util.h b54f689302658fa685ee2df9762f1ca75a85af02 > proton-c/src/extra/scanner.c beb7322bbb128b3b42bcd7585d0b5da833ce01cd > proton-c/src/tests/engine.c 87d8d95100944488d7f13aa5feca3d569e94da3e > > > Diff: https://reviews.apache.org/r/59108/diff/3/ > > > Testing > --- > > Passes all tests that passed previously, several python tests are failing for > me. > > > Thanks, > > Alan Conway > >
Re: Review Request 59108: PROTON-1466: proton-c mixing up links with names that are prefixes
> On May 10, 2017, 7:21 p.m., Andrew Stitcher wrote:
> > I think centralising the comparison is clearly the correct thing to do.
> >
> > I note that you could create a Macro that defines a constant symbol without
> > having to manually count the size:
> >
> > #define MKSYMBOL(X) pn_bytes(sizeof(X)-1, X)
> >
> > used like pn_bytes_equal(something, MKSYMBOL("this-is-a-symbol"));
> > Of course this will only work if X is const char *.
> >
> > Or if you prefer without the quotes in the source:
> >
> > #define MKSYMBOL(X) pn_bytes(sizeof(#X)-1, #X)
> >
> > used like pn_bytes_equal(symbol, MKSYMBOL(this-is-a-symbol));
> >
> > Oddly this is more general in that it kind of always works.
All agreed and done. Called MKSYMBOL PN_BYTES_LITERAL since "symbol" has a
meaning in AMQP distinct from the use of pn_bytes for comparisons of sized
strings.
- Alan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59108/#review174528
---
On May 10, 2017, 6:51 p.m., Alan Conway wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59108/
> ---
>
> (Updated May 10, 2017, 6:51 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Gordon Sim.
>
>
> Bugs: PROTON-1466
> https://issues.apache.org/jira/browse/PROTON-1466
>
>
> Repository: qpid-proton-git
>
>
> Description
> ---
>
> Creating links where one link name is a prefix of the other, e.g. "xx" and
> "xxyy" sometimes caused the links to be confused.
>
>
> Diffs
> -
>
> proton-c/src/core/codec.c 67769ad8ac839242fcc08f008c60cf1b0e8a8bb8
> proton-c/src/core/transport.c 8777318db39e6304ee06c564579a751d0ca2307a
> proton-c/src/core/util.h b54f689302658fa685ee2df9762f1ca75a85af02
> proton-c/src/extra/scanner.c beb7322bbb128b3b42bcd7585d0b5da833ce01cd
> proton-c/src/tests/engine.c 87d8d95100944488d7f13aa5feca3d569e94da3e
>
>
> Diff: https://reviews.apache.org/r/59108/diff/3/
>
>
> Testing
> ---
>
> Passes all tests that passed previously, several python tests are failing for
> me.
>
>
> Thanks,
>
> Alan Conway
>
>
Re: Review Request 59108: PROTON-1466: proton-c mixing up links with names that are prefixes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59108/ --- (Updated May 10, 2017, 8:34 p.m.) Review request for qpid, Andrew Stitcher and Gordon Sim. Changes --- Took on andrew's suggestions Bugs: PROTON-1466 https://issues.apache.org/jira/browse/PROTON-1466 Repository: qpid-proton-git Description --- Creating links where one link name is a prefix of the other, e.g. "xx" and "xxyy" sometimes caused the links to be confused. Diffs (updated) - proton-c/src/core/codec.c 67769ad8ac839242fcc08f008c60cf1b0e8a8bb8 proton-c/src/core/transport.c 8777318db39e6304ee06c564579a751d0ca2307a proton-c/src/core/util.h b54f689302658fa685ee2df9762f1ca75a85af02 proton-c/src/extra/scanner.c beb7322bbb128b3b42bcd7585d0b5da833ce01cd proton-c/src/tests/engine.c 87d8d95100944488d7f13aa5feca3d569e94da3e Diff: https://reviews.apache.org/r/59108/diff/3/ Changes: https://reviews.apache.org/r/59108/diff/2-3/ Testing --- Passes all tests that passed previously, several python tests are failing for me. Thanks, Alan Conway
Re: Review Request 59108: PROTON-1466: proton-c mixing up links with names that are prefixes
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59108/#review174528
---
I think centralising the comparison is clearly the correct thing to do.
I note that you could create a Macro that defines a constant symbol without
having to manually count the size:
#define MKSYMBOL(X) pn_bytes(sizeof(X)-1, X)
used like pn_bytes_equal(something, MKSYMBOL("this-is-a-symbol"));
Of course this will only work if X is const char *.
Or if you prefer without the quotes in the source:
#define MKSYMBOL(X) pn_bytes(sizeof(#X)-1, #X)
used like pn_bytes_equal(symbol, MKSYMBOL(this-is-a-symbol));
Oddly this is more general in that it kind of always works.
proton-c/src/core/util.h
Lines 47 (patched)
I would prefer to see this take 2 pn_bytes_t structs -
pn_bytes_equal(pn_bytes_t a, pn_bytes_t b);
For 2 reasons -
- It would be better named then
- You be able to avoid unecessary strlen calculations. A lot of the time
you are comparing with a constant synbol string anyway where you don't need to
call strlen at all.
- Andrew Stitcher
On May 10, 2017, 6:51 p.m., Alan Conway wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59108/
> ---
>
> (Updated May 10, 2017, 6:51 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Gordon Sim.
>
>
> Bugs: PROTON-1466
> https://issues.apache.org/jira/browse/PROTON-1466
>
>
> Repository: qpid-proton-git
>
>
> Description
> ---
>
> Creating links where one link name is a prefix of the other, e.g. "xx" and
> "xxyy" sometimes caused the links to be confused.
>
>
> Diffs
> -
>
> proton-c/src/core/codec.c 67769ad8ac839242fcc08f008c60cf1b0e8a8bb8
> proton-c/src/core/transport.c 8777318db39e6304ee06c564579a751d0ca2307a
> proton-c/src/core/util.h b54f689302658fa685ee2df9762f1ca75a85af02
> proton-c/src/extra/scanner.c beb7322bbb128b3b42bcd7585d0b5da833ce01cd
> proton-c/src/tests/engine.c 87d8d95100944488d7f13aa5feca3d569e94da3e
>
>
> Diff: https://reviews.apache.org/r/59108/diff/2/
>
>
> Testing
> ---
>
> Passes all tests that passed previously, several python tests are failing for
> me.
>
>
> Thanks,
>
> Alan Conway
>
>
Re: Review Request 59108: PROTON-1466: proton-c mixing up links with names that are prefixes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59108/ --- (Updated May 10, 2017, 6:48 p.m.) Review request for qpid and Gordon Sim. Bugs: PROTON-1466 https://issues.apache.org/jira/browse/PROTON-1466 Repository: qpid-proton-git Description --- Creating links where one link name is a prefix of the other, e.g. "xx" and "xxyy" sometimes caused the links to be confused. Diffs (updated) - proton-c/src/core/codec.c 67769ad8ac839242fcc08f008c60cf1b0e8a8bb8 proton-c/src/core/transport.c 8777318db39e6304ee06c564579a751d0ca2307a proton-c/src/core/util.h b54f689302658fa685ee2df9762f1ca75a85af02 proton-c/src/extra/scanner.c beb7322bbb128b3b42bcd7585d0b5da833ce01cd proton-c/src/tests/engine.c 87d8d95100944488d7f13aa5feca3d569e94da3e Diff: https://reviews.apache.org/r/59108/diff/2/ Changes: https://reviews.apache.org/r/59108/diff/1-2/ Testing --- Passes all tests that passed previously, several python tests are failing for me. Thanks, Alan Conway
Re: Review Request 59108: PROTON-1466: proton-c mixing up links with names that are prefixes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59108/#review174377 --- Ship it! Ship It! - Gordon Sim On May 9, 2017, 8:24 p.m., Alan Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59108/ > --- > > (Updated May 9, 2017, 8:24 p.m.) > > > Review request for qpid and Gordon Sim. > > > Bugs: PROTON-1466 > https://issues.apache.org/jira/browse/PROTON-1466 > > > Repository: qpid-proton-git > > > Description > --- > > Creating links where one link name is a prefix of the other, e.g. "xx" and > "xxyy" sometimes caused the links to be confused. > > > Diffs > - > > proton-c/src/core/codec.c 67769ad8ac839242fcc08f008c60cf1b0e8a8bb8 > proton-c/src/core/transport.c 8777318db39e6304ee06c564579a751d0ca2307a > proton-c/src/core/util.h b54f689302658fa685ee2df9762f1ca75a85af02 > proton-c/src/extra/scanner.c beb7322bbb128b3b42bcd7585d0b5da833ce01cd > proton-c/src/tests/engine.c 87d8d95100944488d7f13aa5feca3d569e94da3e > > > Diff: https://reviews.apache.org/r/59108/diff/1/ > > > Testing > --- > > Passes all tests that passed previously, several python tests are failing for > me. > > > Thanks, > > Alan Conway > >
Review Request 59108: PROTON-1466: proton-c mixing up links with names that are prefixes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59108/ --- Review request for qpid and Gordon Sim. Bugs: PROTON-1466 https://issues.apache.org/jira/browse/PROTON-1466 Repository: qpid-proton-git Description --- Creating links where one link name is a prefix of the other, e.g. "xx" and "xxyy" sometimes caused the links to be confused. Diffs - proton-c/src/core/codec.c 67769ad8ac839242fcc08f008c60cf1b0e8a8bb8 proton-c/src/core/transport.c 8777318db39e6304ee06c564579a751d0ca2307a proton-c/src/core/util.h b54f689302658fa685ee2df9762f1ca75a85af02 proton-c/src/extra/scanner.c beb7322bbb128b3b42bcd7585d0b5da833ce01cd proton-c/src/tests/engine.c 87d8d95100944488d7f13aa5feca3d569e94da3e Diff: https://reviews.apache.org/r/59108/diff/1/ Testing --- Passes all tests that passed previously, several python tests are failing for me. Thanks, Alan Conway
