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

Nightly build broken: Invalid Mode Parameter for Liblouis > 3.7 #51

Open
reiner-dolp opened this issue Nov 26, 2018 · 6 comments
Open

Comments

@reiner-dolp
Copy link
Collaborator

reiner-dolp commented Nov 26, 2018

Since 4 days ago (commit liblouis/liblouis#08084a6 ), passing 0 as mode parameter to lou_translateString gives:

Invalid mode parameter: 0

According to the docs

The mode parameter specifies how the translation should be done. The valid values of mode are
defined in liblouis.h. They are all powers of 2, so that a combined mode can be specified by adding
up different values.

The docs seem wrong, since the _lou_isValidMode is expecting exactly one enum variant to be set:

const int validTranslationModes[] = { noContractions, compbrlAtCursor, dotsIO,
	compbrlLeftCursor, ucBrl, noUndefinedDots, partialTrans };
int
_lou_isValidMode(int mode) {
	for (int i = 0; i < (sizeof(validTranslationModes) / sizeof(*validTranslationModes));
			i++) {
		if (validTranslationModes[i] == mode) {
			return 1;
		}
	}
	return 0;
}

@egli So, am I completely misunderstanding what this code is supposed to do or should _lou_isValidMode be rewritten to accept a bitwise combination of translation modes? Has 0 always been an invalid value?

@reiner-dolp reiner-dolp changed the title Mode Parameter Nighlty build broken: Invalid Mode Parameter for Liblouis > 3.7 Nov 26, 2018
@reiner-dolp reiner-dolp changed the title Nighlty build broken: Invalid Mode Parameter for Liblouis > 3.7 Nightly build broken: Invalid Mode Parameter for Liblouis > 3.7 Nov 26, 2018
@egli
Copy link
Member

egli commented Nov 27, 2018

@reiner-dolp sorry about that. I thought I was clever. I pushed a fix for the worst problem to master. Now the mode can be 0 again. However I guess also bitwise combinations should be allowed. I don't see how this could be achieved with my code atm.
CC @bertfrees

@bertfrees
Copy link
Member

Oh I didn't think about that either.

What about the following?

int isValidMode(int mode) {
    for (int i = 0; i < (sizeof(validTranslationModes) / sizeof(*validTranslationModes)); i++)
        mode &= ~validTranslationModes[i];
    return !mode;
}

By the way shouldn't validTranslationModes be defined static?

@egli
Copy link
Member

egli commented Nov 28, 2018

Thanks @bertfrees for that code.

By the way shouldn't validTranslationModes be defined static?

What is the effect of that?

@egli
Copy link
Member

egli commented Nov 28, 2018

Fixed in liblouis/liblouis@c0820bc (hopefully)

@bertfrees
Copy link
Member

Because now it is extern?

@egli
Copy link
Member

egli commented Nov 28, 2018

Ok, should be static now

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

No branches or pull requests

3 participants