Fix signed vs. unsigned confusion in name sanitisation

Don't try to divide through a signed char when indexing an array, lest
ye try to index off the start of it.

Signed-off-by: Daniel Stone <daniels@collabora.com>

darray: Don't call memcpy() on NULL

The only time we could ever hit this was with count == 0, which seems
unnecessarily pedantic. But OK.

Signed-off-by: Daniel Stone <daniels@collabora.com>

text: NULL-terminate SI mask names

The list should have a NULL sentry. Add one.

testcase: 'interpret KP_Delete+AnyOfOrNaneo(ll)'

Signed-off-by: Daniel Stone <daniels@collabora.com>

xkbcomp: Don't falsely promise from ExprResolveLhs

Every user of ExprReturnLhs goes on to unconditionally dereference the
field return, which can be NULL if xkb_intern_atom fails. Return false
if this is the case, so we fail safely.

testcase: splice geometry data into interp

Signed-off-by: Daniel Stone <daniels@collabora.com>

xkbcomp: Don't explode on invalid virtual modifiers

testcase: 'virtualModifiers=LevelThreC'

Signed-off-by: Daniel Stone <daniels@collabora.com>

xkbcomp: Don't crash on no-op modmask expressions

If we have an expression of the form 'l1' in an interp section, we
unconditionally try to dereference its args, even if it has none.

Signed-off-by: Daniel Stone <daniels@collabora.com>

parser: Don't set more maps when we don't have any

If the scanner indicates that we might have something which looks like a
map, but the parser in fact fails to create that map, we will try to
access the map regardless. Stop doing that.

testcase: 'xkb_keymap {' -> '#kb_keymap'

Signed-off-by: Daniel Stone <daniels@collabora.com>

Bump version to 0.8.1

Signed-off-by: Ran Benita <ran234@gmail.com>

Update NEWS

Signed-off-by: Ran Benita <ran234@gmail.com>

Merge pull request #66 from whot/master

Coverity fixes (one real bug)

action: make a note that we may not null-terminate private strings

Coverity complains that a 7-byte string may not be null-terminated when copied
into act->data (size 7). This is fine, make a note of it.

All the strings in xkeyboard-config only use 6 bytes + null terminator so this
won't be an issue. The server (the only user of these) uses an 8-byte array
and forcibly null-terminates the string, see XkbDDXPrivate().

Everything else treats it as byte-array size 7 anyway so whether it's
null-terminated doesn't matter.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

xkbcomp: fix pointer value for FreeStmt

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

keycodes: don't try to copy zero key aliases

Move the aliases copy to within the (num_key_aliases > 0) block.

Passing info->aliases into this fuction with invalid aliases will
cause log messages but num_key_aliases stays on 0. The key_aliases array
is never allocated and remains NULL. We then loop through the aliases, causing
a null-pointer dereference.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

text: init the target buffer to zero

There's a (theoretical?) path where we might end up strcpy() buf without ever
writing to it. This happens if the mask is nonzero but specifies a modifier
larger than the one in the xkb_mod_set.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

compose: fix infinite loop in parser on some inputs

The parser would enter an infinite loop if an unterminated keysym
literal occurs at EOF.

Found with the afl fuzzer.

Signed-off-by: Ran Benita <ran234@gmail.com>

xkbcomp: fix crash when parsing an xkb_geometry section

xkb_geometry sections are ignored; previously the had done so by
returning NULL for the section's XkbFile, however some sections of the
code do not expect this. Instead, create an XkbFile for it, it will
never be processes and discarded later.

Caught with the afl fuzzer.

Signed-off-by: Ran Benita <ran234@gmail.com>

xkbcomp: fix crashes in the parser when geometry tokens appear

In the XKB format, floats and various keywords can only be used in the
xkb_geometry section. xkbcommon removed support xkb_geometry, but still
parses it for backward compatibility. As part of ignoring it, the float
AST node and various keywords were removed, and instead NULL was
returned by their parsing actions. However, the rest of the code does
not handle NULLs, and so when they appear crashes usually ensue.

To fix this, restore the float AST node and the ignored keywords. None
of the evaluating code expects them, so nice error are displayed.

Caught with the afl fuzzer.

Signed-off-by: Ran Benita <ran234@gmail.com>

xkbcomp: fix stack overflow when evaluating boolean negation

The expression evaluator would go into an infinite recursion when
evaluating something like this as a boolean: `!True`. Instead of
recursing to just `True` and negating, it recursed to `!True` itself
again.

Bug inherited from xkbcomp.

Caught with the afl fuzzer.

Signed-off-by: Ran Benita <ran234@gmail.com>

Add fuzzing infrastructure

Though text formats aren't exactly fuzzer's strong suit, fuzzers can
catch many surface-level bugs.

The fuzz/ directory contains target programs, testcases and dictionaries
to drive the afl fuzzer.

This commit adds a fuzzer for the XKB keymap text format and the Compose
text format. On my slow machine, using a single core, a full cycle of
the XKB fuzzer takes 5 hours. For Compose, it takes a few minutes.

Fuzzing for the other file formats (rules files mostly) will be added
later.

To do some fuzzing, run `./fuzz/fuzz.sh`.

Signed-off-by: Ran Benita <ran234@gmail.com>

Merge pull request #65 from heftig/pc-fix2

meson: Fix xkbcommon-x11.pc Requires versioning

meson: Fix xkbcommon-x11.pc Requires versioning

Old meson expects an array with one dependency per element. Providing a
string containing multiple deps results in only the first dep getting
its whitespace properly applied. As a result, the output was:

    Requires.private: xcb >= 1.10 xcb-xkb>=1.10

And downstream projects failed to find a package named 'xcb-xkb>=1.10'.

Specifying an array of versioned deps results in correct output:

    Requires.private: xcb >= 1.10, xcb-xkb >= 1.10

Fixes #64.

Signed-off-by: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com>

meson: Fix xkbcommon-x11.pc Requires

The meson-generated pkgconfig file was missing Requires and
Requires.private.

[ran: adjust for older Meson versions.]

Signed-off-by: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com>
Signed-off-by: Ran Benita <ran234@gmail.com>

bench: simplify the bench helpers

Trim the API a bit.

Also, just always use gettimeofday(), which is portable. Hopefully the
system clock doesn't change while a benchmark is running.

Signed-off-by: Ran Benita <ran234@gmail.com>

travis: pin meson to an older version so it works with Python 3.4

The CI image only has Python 3.4. It's a bit of a hassle to change this,
so let's pin meson to the version before they bumped their requirement
instead.

I suppose it's not that bad in that it will guarantee compatibility with
older meson over time.

Signed-off-by: Ran Benita <ran234@gmail.com>

keysym-utf: reject out-of-range Unicode codepoints in xkb_keysym_to_utf{8,32}

It used to be UTF-8 was defined for inputs > 0x10FFFF, but nowadays
that's the maximum and a codepoint is encoded up to 4 bytes, not 6.

Fixes: https://github.com/xkbcommon/libxkbcommon/issues/58
Fixes: https://github.com/xkbcommon/libxkbcommon/issues/59
Reported-by: @andrecbarros
Signed-off-by: Ran Benita <ran234@gmail.com>

keysym-utf: replace the Unicode characters for leftanglebracket and rightanglebracket

Looking at leftanglebracket

- The standard[1] does not specify any Unicode value for it.

- The keysym list keysymdef.h in x11proto[2] says U+27E9 MATHEMATICAL
  RIGHT ANGLE BRACKET in a comment.

- The keysym->unicode list in xkbcommon which comes from [3] has U+2329
  LEFT-POINTING ANGLE BRACKET.

- The keysym->unicode list in Xlib[4] has U+2039 SINGLE LEFT-POINTING
  ANGLE QUOTATION MARK.

[1] https://www.x.org/releases/X11R7.7/doc/xproto/x11protocol.html#Legacy_KEYSYMs
[2] https://cgit.freedesktop.org/xorg/proto/x11proto/tree/keysymdef.h
[3] https://www.cl.cam.ac.uk/%7Emgk25/ucs/keysym2ucs.c
[4] https://cgit.freedesktop.org/xorg/lib/libX11/tree/src/xlibi18n/imKStoUCS.c

The symbols we are using, {LEFT,RIGHT}-POINTING ANGLE BRACKET, are
deprecated according to Unicode[5]:

    These characters are deprecated and are strongly discouraged for
    mathematical use because of their canonical equivalence to CJK
    punctuation.

[5] https://www.unicode.org/charts/PDF/U2300.pdf

Hence, switch to the MATHEMATICAL codepoints which seem to be the best
fit.

Fixes: https://github.com/xkbcommon/libxkbcommon/issues/47
Reported-by: @bytensky
Signed-off-by: Ran Benita <ran234@gmail.com>

build/autotools: remove redundant ACLOCAL_AMFLAGS

Will be deprecated in automake 2.0.

Replaced by AC_CONFIG_MACRO_DIR (already present) which is available
since autoconf 2.63.

Signed-off-by: Ran Benita <ran234@gmail.com>

build/meson: fix x11 test/demo compilation with non-standard xcb

The x11 tests/demos did not depend on xcb and xcb-xkb directly, only
indirectly through link_with: libxkbcommon_x11_internal. So linking
worked, but the xcb and xcb-xkb cflags were *not* included when
compiling them. So when using xcb installed in a non-standard location,
what would happen is:

- Library will link with custom xcb and compile with custom xcb headers.
- Test will link with custom xcb and compile with system xcb headers (if
  exist, otherwise fail).

Fixes: https://github.com/xkbcommon/libxkbcommon/issues/57
Reported-by: @remexre
Signed-off-by: Ran Benita <ran234@gmail.com>

build/meson: fix the -Wl,--version-script configure check

Hopefully this fixes compilation on darwin, which doesn't support
version scripts.

Signed-off-by: Ran Benita <ran234@gmail.com>

test/interactive-wayland: replace tabs with spaces

Match the style of all other files.

Signed-off-by: Ran Benita <ran234@gmail.com>

doc: improve the description of the consumed modifier modes

Hopefully it is more understandable now.

https://bugs.freedesktop.org/show_bug.cgi?id=104768

Reported-by: Gatis Paeglis <gatis.paeglis@qt.io>
Signed-off-by: Ran Benita <ran234@gmail.com>

Convert http:// -> https:// where possible

Signed-off-by: Ran Benita <ran234@gmail.com>

x11: check and document the correct range of device IDs

The actual value is 127, not 255.

https://bugs.freedesktop.org/show_bug.cgi?id=104321

Reported-by: Gatis Paeglis <gatis.paeglis@qt.io>
Signed-off-by: Ran Benita <ran234@gmail.com>

Bump version to 0.8.0

Signed-off-by: Ran Benita <ran234@gmail.com>

Update NEWS

Signed-off-by: Ran Benita <ran234@gmail.com>

README,PACKAGING: use the new meson unified command format

Signed-off-by: Ran Benita <ran234@gmail.com>

build: disable -Wdocumentation-deprecated-sync

We will never remove the deprecated functions and there is no real
reason to annoy users into stop using them.

If there *will* be a reason, *then* we will add the attribute.

Fixes: https://github.com/xkbcommon/libxkbcommon/issues/56
Signed-off-by: Ran Benita <ran234@gmail.com>

travis: ensure all tests are valgrind-clean

The tests will fail if they have any memory leaks or perform invalid
accesses.

Signed-off-by: Ran Benita <ran234@gmail.com>

test/x11: properly clean up also when skipping test

To make valgrind happy.

Signed-off-by: Ran Benita <ran234@gmail.com>

compose/parser: be more careful when checking if sequence overrides or duplicates another

Signed-off-by: Ran Benita <ran234@gmail.com>

xkbcomp/ast-build: fix memory leak when appending multi-keysyms

`syms` was not freed.

Signed-off-by: Ran Benita <ran234@gmail.com>

keymap: fix NULL dereference when dumping the default fallback type

The default fallback type uses
    type->level_names = NULL
but the keymap-dump code was not checking this case.

Instead of adding more workarounds and possible bugs (e.g. previous
commit), let's just keep the number of level names separately. This has
the additional advantage retains extraneous level name if someone adds
them for some reason.

Signed-off-by: Ran Benita <ran234@gmail.com>

xkbcomp/types: fix types being assigned the wrong number of levels in some circumstances

The buggy code assigned the number of levels based on the number of
level names in the definition, instead of the actual number of levels!
This would completely break type definitions which do not give names to
levels.

This was not noticed for so long because xkeyboard-config always gives
names to all levels.

This regressed in 61fed8dab9b8e27981f36ffc96666d7376546e30.

Reported-by: Gatis Paeglis <gatis.paeglis@qt.io>
Signed-off-by: Ran Benita <ran234@gmail.com>

keysym: add xkb_keysym_to_{lower,upper} to public API

These can be useful in some odd cases.

There is already an implementation (+ tests) for internal use, so all
that's needed is to export them.

If xkbcommon were to provide a way to convert a Unicode codepoint to a
keysym, this could have been implemented externally as follows:

    uint32_t codepoint = xkb_keysym_to_utf32(keysym);
    uint32_t upper_codepoint = my_unicode_library_to_upper(codepoint);
    xkb_keysym_t upper_keysym = theoretical_xkb_keysym_from_utf32(upper_codepoint);

However keysym -> codepoint is not injective so such a function is not
possible strictly speaking.

Signed-off-by: Ran Benita <ran234@gmail.com>

test/data: sync with xkeyboard-config 2.22

Some tweaks to the de(neo) keyseq tests were required. It seems to have
improved.

Signed-off-by: Ran Benita <ran234@gmail.com>

Merge pull request #53 from whitslack/missing-ucs-codes

keysym-utf: Add missing codes for signifblank and permille keysyms

keysym-utf: Add missing codes for signifblank and permille keysyms

build/meson: require meson >= 0.41.0

With previous versions, the compilation fails with linker errors.

Fixes https://github.com/xkbcommon/libxkbcommon/issues/52
Reported-by: @rezso
Signed-off-by: Ran Benita <ran234@gmail.com>

expr: paper over a maybe-uninitialized warning

Signed-off-by: Ran Benita <ran234@gmail.com>

test/interactive-wayland: avoid unused function warning due to configuration

test/interactive-wayland.c:95:1: warning: ‘set_cloexec_or_close’ defined but not used [-Wunused-function]

Signed-off-by: Ran Benita <ran234@gmail.com>

build: add missing configure function checks for test/interactive-wayland

Signed-off-by: Ran Benita <ran234@gmail.com>

xkbcommon-compose.h: change recommended locale fallback code to treat empty string same as unset

The previous code would cause failures to find the Compose file if one
of the environment variables LC_ALL, LC_CTYPE or LANG are set to the
empty string.

The description of the fallback procedure in loclale(7) talks about
"non-null environment variable"; I interpreted this to mean the
environment variable is unset, but it actually means unset or empty (I
verified this by looking at what glibc and musl do).

A recent bug in systemd https://github.com/systemd/systemd/issues/6407
exposed this issue. It causes these these variables to be set to the
empty string in TTY sessions.

Reported by "doodoo" in https://bbs.archlinux.org/viewtopic.php?id=228658

Signed-off-by: Ran Benita <ran234@gmail.com>

Bump version to 0.7.2

Signed-off-by: Ran Benita <ran234@gmail.com>

Update NEWS

Signed-off-by: Ran Benita <ran234@gmail.com>

build/autotools: fix doc installation

Signed-off-by: Ran Benita <ran234@gmail.com>

test/symbols-leak-test.bash: make it easier to read

Signed-off-by: Ran Benita <ran234@gmail.com>

build/autotools: fix symbol versioning configure check

Regressed in 4309735. I thought `$top_srcdir` works in configure too,
apparently not!

See: https://github.com/xkbcommon/libxkbcommon/issues/50#issuecomment-319693694

Signed-off-by: Ran Benita <ran234@gmail.com>

build/autotools: make doxygen's input relative

Was meant to be a part of 41bea9a.

Signed-off-by: Ran Benita <ran234@gmail.com>

build/autotools: dist the meson build files

So that it's possible to build with meson from autotools-generated
tarballs.

Signed-off-by: Ran Benita <ran234@gmail.com>

build/autotools: install documentation under html/

To be consistent with the meson build; also makes more sense as doxygen
can generate more than html (though we currently are not doing that).

Signed-off-by: Ran Benita <ran234@gmail.com>

build: make doxygen run from the source tree

I couldn't find any other way to make this work!

Signed-off-by: Ran Benita <ran234@gmail.com>

Merge pull request #49 from bluetech/meson

build: add meson build system

build: add meson build system

Meson is easier to maintain, much faster, encourages better practices,
and is not built on a pile of shell scripts.

The autotools build system is kept intact for now, in order to ease the
migration. The intention is to remove it sooner rather than later, if
all goes well.

Run `meson build && mesonconf build` to see the configuration options
for the new system. Conversion should be straightforward. Environment
variables like CFLAGS work the same.

If meson is used, xorg-util-macros is not required.

In terms of functionality the two systems have about the same
capabilities. Here are some differences I noticed:

- Meson uses `-g` by default, autotools uses `-g -O2`.

- In autotools the default behavior is to install both static and shared
  versions of the libraries. In meson the user must choose exactly one
  (using -Ddefault_library=static/shared).

  It is possible to workaround if needed (install twice...), but
  hopefully meson will add the option in the future.

- Autotools has builtin ctags/cscope targets, meson doesn't.

  Easy to run the tools directly.

- Meson has builtin benchmarks target. Handy.

- Meson has builtin support for sanitizers/clang-analyzer/lto/pgo/
  coverage etc. Also handy.

Signed-off-by: Ran Benita <ran234@gmail.com>

build: remove Android.mk

I don't think it will work with meson, and I also don't think anyone is
using it.

Signed-off-by: Ran Benita <ran234@gmail.com>

build: remove -uninstalled pkg-config files

Meson doesn't support them directly, and I don't think anyone is using
them.

Signed-off-by: Ran Benita <ran234@gmail.com>

travis: do the build out-of-tree

To catch problems like the previous commit.

Signed-off-by: Ran Benita <ran234@gmail.com>

build: fix out-of-tree build

The change in d44ba48 removed -I$(top_builddir)/src/xkbcomp, but this is
needed in order to find the generated parser.h file which is put in the
build dir.

I also added -I$(top_builddir)/src in order to match the meson behavior.

Fixes https://github.com/xkbcommon/libxkbcommon/issues/50

Signed-off-by: Ran Benita <ran234@gmail.com>

build: use top_srcdir consistently

Signed-off-by: Ran Benita <ran234@gmail.com>

build: change doxygen target to be properly dependency-based

This hackery (thanks libinput) is clearer and more precise than the
previous hackery.

Signed-off-by: Ran Benita <ran234@gmail.com>

build: move custom targets to scripts/ and remove from makefile

These scripts generate source code that is committed to git and hence do
not really belong in the build system. A maintainer runs them as needed.

Signed-off-by: Ran Benita <ran234@gmail.com>

build: rewrite the update-keysyms sed script in python

A bit more manageable this way, and the other part of the target is
already using python.

The output is the same, except I removed the reference to Makefile.am.

Signed-off-by: Ran Benita <ran234@gmail.com>

build: remove unneeded preprocessor include flags

Better to avoid these unexpected include paths.

Signed-off-by: Ran Benita <ran234@gmail.com>

build: explain why we don't AC_DEFINE a couple of constants

Signed-off-by: Ran Benita <ran234@gmail.com>

x11/keymap,test/interactive-evdev: fix a couple of clang-analyzer warnings

From my analysis these values cannot be null, but the analyzer cannot
see this. So assert it.

Signed-off-by: Ran Benita <ran234@gmail.com>

test/x11comp: fix compiler warnings

Signed-off-by: Ran Benita <ran234@gmail.com>

Merge pull request #48 from namandixit/patch-1

Fixed a minor bug in error detection in Wayland test

Fixed a minor bug in error detection in Wayland test

Merge pull request #46 from jwrdegoede/master

Sync Keysyms with recent xproto additions

Sync Keysyms with recent xproto additions

xproto recently has been extended with 4 new keysyms:
XF86XK_Keyboard
XF86XK_WWAN
XF86XK_RFKill
XF86XK_AudioPreset

This commit is the result of running "make update-keysyms" on a system
with the updated xproto installed.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>

doc/compat: correct the XKB protocol version from 1.1 to 1.0

There is no XKB 1.1!

Thanks to Oded Arbel for catching this.

Signed-off-by: Ran Benita <ran234@gmail.com>

test/interactive-x11: handle NULL from xcb_wait_for_event

Can happen in cases like:
- There was an error between the error check and the call.
- The internal poll() fails.

Signed-off-by: Ran Benita <ran234@gmail.com>

state: cure boolean blindness in the filter functions' result

Makes it a little easier to understand the filters.

Signed-off-by: Ran Benita <ran234@gmail.com>

state: remove unneeded NULL check

xkb_filter_new() cannot return NULL.

Signed-off-by: Ran Benita <ran234@gmail.com>

state: reorder new() functions before the set() functions in the code

So that they may be read more naturally in chronological order.

Signed-off-by: Ran Benita <ran234@gmail.com>

test/x11comp: be a bit more careful with kill()

We did it correctly but better be safe and appease clang.

Signed-off-by: Ran Benita <ran234@gmail.com>

test/interactive-wayland: mark a local function static

Signed-off-by: Ran Benita <ran234@gmail.com>

test/interactive-wayland: handle unrecognized SHM format

The enum seems large, and we don't handle all of the values in it.
Previously if we got an unrecognized SHM format we would use an
uninitialized `stride`.

Signed-off-by: Ran Benita <ran234@gmail.com>

test/interactive-wayland: fix uninitialized `ret` in error path

Signed-off-by: Ran Benita <ran234@gmail.com>

Merge pull request #45 from fooishbar/xdg-shell-v6

Wayland: xdg-shell v6 support

interactive-wayland: Port to xdg-shell v6

Mutter only implements v6 now, and Weston also implements that. Port
interactive-wayland to this so people can keep on using it.

Signed-off-by: Daniel Stone <daniels@collabora.com>

Add explicit fallthrough case statements

When we fall through to another label in a case, add an explicit comment
noting so, to quiet GCC 7's warnings.

Signed-off-by: Daniel Stone <daniels@collabora.com>

Bump version to 0.7.1

Signed-off-by: Ran Benita <ran234@gmail.com>

Update NEWS

Signed-off-by: Ran Benita <ran234@gmail.com>

compose/doc: note that it is safe to pass the result of getenv() as locale

See the NOTES section of getenv(3). Somewhat obscure but it doesn't hurt
to reassure the readers who know about this.

Signed-off-by: Ran Benita <ran234@gmail.com>

compose: remove the keysym_from_name cache

The hit rate is high, but either the cache is slow or the function is
not fast enough -- the cache no longer holds its weight, leading only to
very modest improvements. If it's the former, it can definitely be
improved, the code is very dumb (though it worked just as well as any
other I tried back then). But instead, let's just kill it.

Signed-off-by: Ran Benita <ran234@gmail.com>

doc: note that XKB_KEYSYM_CASE_INSENSITIVE does C folding only

and not locale-dependent.

Signed-off-by: Ran Benita <ran234@gmail.com>

keysym: fix locale dependence in xkb_keysym_from_name()

We currently use strcasecmp, which is locale-dependent. In particular,
one well-known surprise even if restricted just ASCII input is found in
the tr_TR (Turkish) locale, see e.g.
https://msdn.microsoft.com/en-us/library/ms973919.aspx#stringsinnet20_topic5

We have known to avoid locale-dependent functions before, but in this
case, we forgot.

Fix it by implementing our own simple ASCII-only strcasecmp/strncasecmp.
Might have been possible to use strcasecmp_l() with the C locale, but
went the easy route.

Side advantage is that even this non-optimized version is faster than
the optimized libc one (__strcasecmp_l_sse42) since it doesn't need to
do the locale stuff. xkb_keysym_from_name(), which uses strcasecmp
heavily, becomes faster, and so for example Compose file parsing, which
uses xkb_keysym_from_name() heavily, becomes ~20% faster.

Resolves https://github.com/xkbcommon/libxkbcommon/issues/42
Signed-off-by: Ran Benita <ran234@gmail.com>

utils: rename popcount to avoid conflict in NetBSD

Resolves https://github.com/xkbcommon/libxkbcommon/issues/41
Signed-off-by: Ran Benita <ran234@gmail.com>

Bump version to 0.7.0

Signed-off-by: Ran Benita <ran234@gmail.com>

Update NEWS

Signed-off-by: Ran Benita <ran234@gmail.com>