Re: Maybe too strict check for frontend/backend name conflicts

On Fri, 2 Nov 2007, Willy Tarreau wrote:

> Hi Krzysztof !

Hi Willy,

>> Below you can find
>> the stripped version of mentioned patch, with unready parts removed, but
>> extended to do what I hope you had expected. RFC quality so not for commit
>> this time.
>>
>> This patch:
>> - adds proxy_mode_str() similar to proxy_type_str()
>> - adds a generic findproxy function used with
>> default_backend/senbe/use_backed
>> - rewrite default_backend/senbe/use_backed to use introduced findproxy()
>> - relaxes duplicated proxy check
>
> I like it. It makes both the parsing and the checks cleaner. It reminds
> me that we have to change the capabilities displaying from "%X" to "%s"
> with a call to proxy_type_str(). When I first saw the error messages, I
> got "conflicting names for proxy capabilities 7/7" or something like this,
> and eventhough I know what it means, I cannot expect normal users to be
> aware of the internals.

OK.

> Also, you might have noticed that I'm trying to de-centralize the parsing
> to various files, in the hope that later we'll be able to easily load
> modules which add keywords. Since you're adding new functions such as
> findproxy, see if you can put them in proxy.c.

Right, it is obviously wrong. I have no idea why I put it here, especially that proxy_mode_str is in the proper place.

> Take a look at how I managed to pass the error messages in return from
> dumpstats.c,

OK, found it.

> or even in backend.c (backend_parse_balance, in latest master-git).
There is no backend_parse_balance. :( Checked both 0e21b201e6892c25e8264e43ceb12fed6d0c5e82 (1.3.13) and 85130941e7ba66656e302a68a211c3834d7d5a93 (master).

> I've not found the perfect interface to all parsing functions yet, but
> I'm sure we'll soon have something standard for use with all
> sub-components.
>
> It will also greatly improve error messages report since each level can
> complete the message with more information, and the last level can put
> the line number which we don't always have. I'll have to update ACLs to
> use the same interface so that we can at last get human-readable error
> messages for incorrectly written ACLs.

How about adding a global "char errstr[BUFSIZE]"? Each function can then easily return a appropriate error message. No need to pass additional buffers and lengths.

Best regards,

                         Krzysztof Olêdzki Received on 2007/11/02 11:43