Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the data type explicit in transport mock #1914

Merged
merged 1 commit into from
Mar 10, 2018

Conversation

mochrul
Copy link
Contributor

@mochrul mochrul commented Mar 4, 2018

Previously there was a heuristic in that object
which tried to detect if the data is a string or an
error code. But this heuristic was failing in GNU Hurd.
With this patch, the heuristic is replaced with a
type field in a structure.

This patch fixes issue #1912

I welcome every suggestion and, as you can see, the giving name is still not my cup of tea, so if somebody knows a better name for the variables or the new structure, I would be more than happy to rename them.

@kira-syslogng
Copy link
Contributor

This user does not have permission to start the build. Can one of the admins verify this patch and start the build? (admin please type: ok to test)

@MrAnno
Copy link
Collaborator

MrAnno commented Mar 4, 2018

@kira-syslogng ok to test

@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

@gaborznagy
Copy link
Collaborator

Hi @mochrul!

Just a question out of curiosity:
Which version of syslog-ng will you use/need on Hurd? Just because you state the problem is on 3.6 and you are patching it in upstream.

@mochrul
Copy link
Contributor Author

mochrul commented Mar 7, 2018

If you check the bug ticket you can see that that is just a replica.
The original ticket is dated back to 2014 when 3.6 is not yet released officially (or just barely released). I just copied all the information from that ticket into this new, because that ticket is closed and I was not able to reopen it. And because the bug is more than 3 years old, I think fixing this in HEAD is enough.

Also, I do not use Hurd but I'm the Debian maintainer of the package and Debian wants to compile it on Hurd. Most of my bug reports and patches are related to the Debian packaging.

@gaborznagy
Copy link
Collaborator

gaborznagy commented Mar 8, 2018 via email

self->iov_cnt++;
g_assert(self->value_cnt < sizeof(self->value) / sizeof(self->value[0]));

if (length == -2)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is kinda hard to connect this -2 value and the one in the LTM_INJECT_ERROR macro.
Would you create a constant for this ?

#define LTM_INJECT_ERROR_LENGTH -2
#define LTM_INJECT_ERROR(err)   (GUINT_TO_POINTER(err)), LTM_INJECT_ERROR_LENGTH

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, one small thing. Your update does not compile because you forget the -2 from the LTM_InJECT_ERROR_LENGTH definition.

@kira-syslogng
Copy link
Contributor

This user does not have permission to start the build. Can one of the admins verify this patch and start the build? (admin please type: ok to test)

@kira-syslogng
Copy link
Contributor

success

@mochrul mochrul force-pushed the hurd_compile_fix branch from aad4eb8 to 12c7298 Compare March 9, 2018 21:02
@kira-syslogng
Copy link
Contributor

Build FAILURE, the tests were executed on test branch: master and test suite: functions

@kira-syslogng
Copy link
Contributor

success

@mochrul mochrul force-pushed the hurd_compile_fix branch 2 times, most recently from 16dc6f4 to 8e2b8c9 Compare March 10, 2018 07:58
@kira-syslogng
Copy link
Contributor

success

@kira-syslogng
Copy link
Contributor

Build FAILURE, the tests were executed on test branch: master and test suite: functions

@kira-syslogng
Copy link
Contributor

success

Previously there were a heuristic in that object
which tried to detect if the data is a string or an
error code. But this heuristic was failing in GNU Hurd.
With this patch the heuristic is replaced with a
type field in a structure.
@mochrul mochrul changed the title Make the the data type explicit in transport mock Make the data type explicit in transport mock Mar 10, 2018
@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

@kira-syslogng
Copy link
Contributor

success

@Kokan Kokan merged commit a9f3d67 into syslog-ng:master Mar 10, 2018
@mochrul mochrul deleted the hurd_compile_fix branch March 18, 2018 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants