[Libwebsockets] Recent buggy changes

Andrey Pokrovskiy wonder.mice at gmail.com
Wed May 6 02:33:59 CEST 2015


> Otherwise I think we were better off how it was.
> I assume user code prior to this with CMAKE_BUILD will just continue to work fine.

To use the previous version of the library correctly CMAKE_BUILD
_must_ be defined. It it worked without CMAKE_BUILD defined you just
got lucky.

If CMAKE_BUILD _must_ be defined, then it doesn't make sense to check
whether it defined or not.

> I think the root cause of the current situation is I accepted some patchbombs I should have asked to be split up.

It already was a _separate_ patch :)


On Tue, May 5, 2015 at 5:20 PM, Andy Green <andy.green at linaro.org> wrote:
>
> On 6 May 2015 07:40, "Andrey Pokrovskiy" <wonder.mice at gmail.com> wrote:
>>
>> You have to ALWAYS include lws_config.h in the beginning of
>> libwebsockets.h because that's how libwebsockets.h knows about options
>> with which the library was compiled. If you look into libwebsockets.h
>> you will see it uses defines from lws_config.h, such as
>> LWS_NO_EXTENSIONS, LWS_OPENSSL_SUPPORT, LWS_USE_LIBEV.
>>
>> Before that patch it was basically broken and to use it correctly you
>> have to define CMAKE_BUILD in your project manually to workaround
>> that.
>>
>> But I agree that it's completely inappropriate to have such generic
>> defines in library public header. I even added a comment for that:
>>
>> /* That's a bad idea since it will leak all internal defines outside */
>> #include "lws_config.h"
>
> Yeah.  It's better if you don't say, "it's a bad idea to stick my hand in
> the fire" and then stick your hand in the fire as if that absolved anyone
> from feeling the pain though.
>
>> I suppose the right solution is to split lws_config.h on two - public
>> (LWS_xxx things) and private (all other things). Or just prefix
>> everything with "LWS_". But any solution probably will have some
>> "compatibility" issues, so chances are things will remain the way they
>> are.
>
> I think the root cause of the current situation is I accepted some
> patchbombs I should have asked to be split up.
>
>> On Tue, May 5, 2015 at 1:19 PM, Roger Light <andy.green at linaro.org> wrote:
>> > Hi there,
>> >
>> > I believe that a recent changeset introduced some undesirable changes
>> > to libwebsockets. I'm not sure what the best solution is, hence this
>> > email.
>> >
>> > The change is:
>> >
>> >
>> > https://github.com/warmcat/libwebsockets/commit/04da2ccd1e8c5b582c4e2a77ee53f929ae8f22a0
>> >
>> > "Always include lws_config.h since now we have only CMake build"
>> >
>> > This means that libwebsockets.h always #includes lws_config.h, whereas
>> > before it was only included if CMAKE_BUILD was defined - this means
>> > only when the library itself was being built in practice. From what I
>> > can see, lws_config.h includes a mix of information that is useful to
>> > people using lws - such as whether it was compiled with cyassl support
>> > - and lots of things that aren't really of any interest.
>> >
>> > One of these things is the "#define VERSION" macro. This means that
>> > lws is claiming ownership of the VERSION macro, which seems a bit
>> > impolite for a library to do :) The shame of it is that the changeset
>> > acknowledges that the change is going to cause problems.
>> >
>> > I think that these is a lot of leftover cruft from autoconf days, but
>> > I don't know everything that can be cut. I think that if it is
>> > possible to have lws_config.h for public information about the lws
>> > build, and lws_config_private.h for things that lws needs for its own
>> > build that would be a reasonable solution.
>
> It sounds good to me if someone will implement it.  Otherwise I think we
> were better off how it was.
>
> I assume user code prior to this with CMAKE_BUILD will just continue to work
> fine.
>
> -Andy
>
>> > Any thoughts?
>> >
>> > Cheers,
>> >
>> > Roger
>> > _______________________________________________
>> > Libwebsockets mailing list
>> > Libwebsockets at ml.libwebsockets.org
>> > http://ml.libwebsockets.org/mailman/listinfo/libwebsockets



More information about the Libwebsockets mailing list