Re: 51Degrees Trie Update Patch
Hi Ben, On Mon, Aug 08, 2016 at 03:25:04PM +, Ben Shillito wrote: > Hi Willy, > > Yes I agree, this is a problem. > I have attached a patch with a change to the readme which explains that that > the correct version must be pulled if using <=1.6. (...) +and checkout the latest compatible version (this step is only neccessary when +using version 1.6 or earlier) : + +git checkout tags/v3.2.5.3 + Well, if we mention a tag in the readme for stable branches, that means that this code will never ever receive fixes anymore ? That doesn't sound very wise, as I guess this doc will have to change next time you face a bug. Why don't you simply create a branch named with the previous API ? That way when you want to merge fixes you simply put them into that branch and you don't have to fear about the many people stuck to your old tag. Regards, Willy
RE: 51Degrees Trie Update Patch
Hi Willy,
Yes I agree, this is a problem.
I have attached a patch with a change to the readme which explains that that
the correct version must be pulled if using <=1.6.
Regards,
Ben.
Ben Shillito
Developer
E: b...@51degrees.com
T: @51Degrees
-Original Message-
From: Willy Tarreau [mailto:w...@1wt.eu]
Sent: 04 August 2016 16:43
To: Ben Shillito <b...@51degrees.com>
Cc: haproxy@formilux.org
Subject: Re: 51Degrees Trie Update Patch
Hi Ben,
sorry I missed this one.
On Wed, Jul 06, 2016 at 11:25:17AM +, Ben Shillito wrote:
> Attached is a patch submission which makes changes to the 51Degrees
> Trie implementation to work with recent changes to
> github.com/51Degrees/Device-Detection/src/trie/51Degrees.c.
Thanks, but that makes me realize something really bad :
--- a/src/51d.c
+++ b/src/51d.c
@@ -235,7 +235,7 @@ static void _51d_set_device_offsets(struct sample *smp)
(global._51degrees.header_names + index)->len,
msg->chn->buf->p, idx, ) == 1) {
(offsets->firstOffset + offsets->size)->httpHeaderOffset =
*(global._51degrees.header_offsets + index);
-(offsets->firstOffset + offsets->size)->deviceOffset =
fiftyoneDegreesGetDeviceOffset(ctx.line + ctx.val);
+(offsets->firstOffset + offsets->size)->deviceOffset =
+fiftyoneDegreesGetDeviceOffset(_51degrees.data_set, ctx.line +
+ctx.val);
offsets->size++;
}
As you can see the API was changed in the master branch which is also the one
which was documented in the stable version as being the one to pull from. This
is really not cool because stable users who apply what is written in the readme
will get a build failure. Could you please at least provide an update for the
build procedure in the readme that we can backport to 1.6 to reference a branch
that continues to work for these users ? While it will not fix the problem for
existing users, at least they can find a hint in newer releases about how to
fix their issues.
Thanks,
Willy
This email and any attachments are confidential and may also be privileged. If
you are not the named recipient, please notify the sender immediately and do
not disclose, use, store or copy the information contained herein. This is an
email from 51Degrees.mobi Limited, 5 Charlotte Close, Reading. RG47BY. T: +44
118 328 7152; E: i...@51degrees.com; 51Degrees.mobi Limited t/as 51Degrees.
0001-DOC-Updated-51Degrees-section-of-ReadMe-to-reflect-c.patch
Description: 0001-DOC-Updated-51Degrees-section-of-ReadMe-to-reflect-c.patch
Re: 51Degrees Trie Update Patch
Hi Ben,
sorry I missed this one.
On Wed, Jul 06, 2016 at 11:25:17AM +, Ben Shillito wrote:
> Attached is a patch submission which makes changes to the 51Degrees Trie
> implementation to work with recent changes to
> github.com/51Degrees/Device-Detection/src/trie/51Degrees.c.
Thanks, but that makes me realize something really bad :
--- a/src/51d.c
+++ b/src/51d.c
@@ -235,7 +235,7 @@ static void _51d_set_device_offsets(struct sample *smp)
(global._51degrees.header_names + index)->len,
msg->chn->buf->p, idx, ) == 1) {
(offsets->firstOffset +
offsets->size)->httpHeaderOffset = *(global._51degrees.header_offsets + index);
- (offsets->firstOffset + offsets->size)->deviceOffset =
fiftyoneDegreesGetDeviceOffset(ctx.line + ctx.val);
+ (offsets->firstOffset + offsets->size)->deviceOffset =
fiftyoneDegreesGetDeviceOffset(_51degrees.data_set, ctx.line + ctx.val);
offsets->size++;
}
As you can see the API was changed in the master branch which is also the
one which was documented in the stable version as being the one to pull
from. This is really not cool because stable users who apply what is written
in the readme will get a build failure. Could you please at least provide an
update for the build procedure in the readme that we can backport to 1.6 to
reference a branch that continues to work for these users ? While it will not
fix the problem for existing users, at least they can find a hint in newer
releases about how to fix their issues.
Thanks,
Willy
