Merge pull request #1506 from houqp/houqp-master

add static analysis for Lua

.luacheckrc

@@ -0,0 +1,91 @@
+unused_args = false
+std = "luajit"
+
+globals = {
+    "G_reader_settings",
+}
+
+read_globals = {
+    "DRCOUNTMAX",
+    "DHINTCOUNT",
+    "DFULL_SCREEN",
+    "DSCROLL_MODE",
+    "DGLOBALGAMMA",
+    "DRENDER_MODE",
+    "DGLOBAL_CACHE_SIZE_MINIMUM",
+    "DGLOBAL_CACHE_FREE_PROPORTION",
+    "DGLOBAL_CACHE_SIZE_MAXIMUM",
+    "DBACKGROUND_COLOR",
+    "DOUTER_PAGE_COLOR",
+    "DCREREADER_VIEW_MODE",
+    "DSHOWOVERLAP",
+    "DSHOWHIDDENFILES",
+    "DLANDSCAPE_CLOCKWISE_ROTATION",
+    "DCREREADER_TWO_PAGE_THRESHOLD",
+    "DOVERLAPPIXELS",
+    "FOLLOW_LINK_TIMEOUT",
+    "DTAP_ZONE_MENU",
+    "DTAP_ZONE_CONFIG",
+    "DTAP_ZONE_MINIBAR",
+    "DTAP_ZONE_FORWARD",
+    "DTAP_ZONE_BACKWARD",
+    "DTAP_ZONE_BOOKMARK",
+    "DTAP_ZONE_FLIPPING",
+    "DDOUBLE_TAP_ZONE_NEXT_CHAPTER",
+    "DDOUBLE_TAP_ZONE_PREV_CHAPTER",
+    "DCHANGE_WEST_SWIPE_TO_EAST",
+    "DCHANGE_EAST_SWIPE_TO_WEST",
+    "DKOPTREADER_CONFIG_FONT_SIZE",
+    "DKOPTREADER_CONFIG_TEXT_WRAP",
+    "DKOPTREADER_CONFIG_TRIM_PAGE",
+    "DKOPTREADER_CONFIG_DETECT_INDENT",
+    "DKOPTREADER_CONFIG_DEFECT_SIZE",
+    "DKOPTREADER_CONFIG_PAGE_MARGIN",
+    "DKOPTREADER_CONFIG_LINE_SPACING",
+    "DKOPTREADER_CONFIG_RENDER_QUALITY",
+    "DKOPTREADER_CONFIG_AUTO_STRAIGHTEN",
+    "DKOPTREADER_CONFIG_JUSTIFICATION",
+    "DKOPTREADER_CONFIG_MAX_COLUMNS",
+    "DKOPTREADER_CONFIG_CONTRAST",
+    "DKOPTREADER_CONFIG_WORD_SPACINGS",
+    "DKOPTREADER_CONFIG_DEFAULT_WORD_SPACING",
+    "DKOPTREADER_CONFIG_DOC_LANGS_TEXT",
+    "DKOPTREADER_CONFIG_DOC_LANGS_CODE",
+    "DKOPTREADER_CONFIG_DOC_DEFAULT_LANG_CODE",
+    "DCREREADER_CONFIG_FONT_SIZES",
+    "DCREREADER_CONFIG_DEFAULT_FONT_SIZE",
+    "DCREREADER_CONFIG_MARGIN_SIZES_SMALL",
+    "DCREREADER_CONFIG_MARGIN_SIZES_MEDIUM",
+    "DCREREADER_CONFIG_MARGIN_SIZES_LARGE",
+    "DCREREADER_CONFIG_LIGHTER_FONT_GAMMA",
+    "DCREREADER_CONFIG_DEFAULT_FONT_GAMMA",
+    "DCREREADER_CONFIG_DARKER_FONT_GAMMA",
+    "DCREREADER_CONFIG_LINE_SPACE_PERCENT_SMALL",
+    "DCREREADER_CONFIG_LINE_SPACE_PERCENT_MEDIUM",
+    "DCREREADER_CONFIG_LINE_SPACE_PERCENT_LARGE",
+    "DCREREADER_PROGRESS_BAR",
+    "DMINIBAR_TOC_MARKER_WIDTH",
+    "DMINIBAR_HEIGHT",
+    "DMINIBAR_CONTAINER_HEIGHT",
+    "DMINIBAR_FONT_SIZE",
+    "DGESDETECT_DISABLE_DOUBLE_TAP",
+    "DAUTO_SAVE_PAGING_COUNT",
+    "DDICT_FONT_SIZE",
+    "FRONTLIGHT_SENSITIVITY_DECREASE",
+    "DALPHA_SORT_CASE_INSENSITIVE",
+    "SEARCH_LIBRARY_PATH",
+    "SEARCH_LIBRARY_PATH2",
+    "SEARCH_CASESENSITIVE",
+    "SEARCH_AUTHORS",
+    "SEARCH_TITLE",
+    "SEARCH_TAGS",
+    "SEARCH_SERIES",
+    "SEARCH_PATH",
+    "KOBO_LIGHT_ON_START",
+    "KOBO_SCREEN_SAVER",
+    "KOBO_SCREEN_SAVER_LAST_BOOK",
+    "NETWORK_PROXY",
+    "cre",
+    "lfs",
+    "lipc",
+}

.travis.yml

@@ -22,13 +22,13 @@ install:
     # install an older version of lua_cliargs for busted v1.10.0
     - sudo luarocks install lua_cliargs 2.1-2
     # recent versions of busted may cause some weird segmentation faults
-    - git clone https://github.com/Olivine-Labs/busted/
+    - travis_retry git clone https://github.com/Olivine-Labs/busted/
     - cd busted && git checkout v1.10.0 && sudo luarocks make && cd ..
-    - sudo luarocks install luacov
-    - sudo luarocks install luacov-coveralls --server=http://rocks.moonscript.org/dev
+    - travis_retry sudo luarocks install luacov
+    - travis_retry sudo luarocks install luacov-coveralls --server=http://rocks.moonscript.org/dev
 
 script:
-    - make fetchthirdparty all
+    - travis_retry make fetchthirdparty all
     - sudo cp base/build/*/luajit /usr/bin/
     - make testfront
 

Makefile

@@ -251,4 +251,9 @@ pot:
 po:
 	$(MAKE) -i -C l10n bootstrap pull
 
+static-check:
+	@if which luacheck > /dev/null; then luacheck frontend; else \
+		echo "[!] luacheck not found. "\
+		"you can install it with 'luarocks install luacheck'"; fi
+
 .PHONY: test

README.md

@@ -74,7 +74,7 @@ Your GCC should be at least of version 4.7 for both native and cross compiling.
 
 Users of Debian and Ubuntu can install the required packages using:
 ```
-sudo apt-get install build-essential libtool gcc-multilib \
+sudo apt-get install build-essential libtool gcc-multilib libffi-dev \
 patch wget unzip git autoconf subversion cmake nasm libsdl1.2-dev
 ```
 
@@ -181,6 +181,11 @@ To run unit tests in KOReader:
 make test
 ```
 
+To run Lua static-analysis for KOReader:
+```
+make statick-check
+```
+
 You may need to checkout the [travis config file][travis-conf] to setup up
 a proper testing environment. Briefly, you need to install `luarocks` and
 then install `busted` with `luarocks`. The "eng" language data file for

frontend/apps/filemanager/filemanager.lua

@@ -109,7 +109,6 @@ function FileManager:init()
                 {
                     text = _("Delete"),
                     callback = function()
-                        local path = util.realpath(file)
                         local ConfirmBox = require("ui/widget/confirmbox")
                         UIManager:close(self.file_dialog)
                         UIManager:show(ConfirmBox:new{

frontend/apps/filemanager/filemanagerfilesearcher.lua

@@ -14,7 +14,7 @@ local _ = require("gettext")
 
 local FileSearcher = InputContainer:new{
     search_dialog = nil,
-    
+
     --filesearcher
     -- state buffer
     dirs = {},
@@ -22,7 +22,7 @@ local FileSearcher = InputContainer:new{
     results = {},
     items = 0,
     commands = nil,
-    
+
     --filemanagersearch
     use_previous_search_results = false,
     lastsearch = nil,
@@ -33,7 +33,7 @@ function FileSearcher:readDir()
     DEBUG("self.path", self.path)
     self.files = {}
     while #self.dirs ~= 0 do
-        new_dirs = {}
+        local new_dirs = {}
         -- handle each dir
         for __, d in pairs(self.dirs) do
             -- handle files in d
@@ -104,7 +104,7 @@ end
 function FileSearcher:showSearch()
     local dummy = self.search_value
     self.search_dialog = InputDialog:new{
-        title = _("Search for books by filename"),             
+        title = _("Search for books by filename"),
         input = self.search_value,
         buttons = {
             {
@@ -139,7 +139,6 @@ function FileSearcher:showSearch()
 end
 
 function FileSearcher:showSearchResults()
-    local ReaderUI = require("apps/reader/readerui")
     local menu_container = CenterContainer:new{
         dimen = Screen:getSize(),
     }

frontend/apps/filemanager/filemanagerhistory.lua

@@ -6,7 +6,6 @@ local UIManager = require("ui/uimanager")
 local DocSettings = require("docsettings")
 local Menu = require("ui/widget/menu")
 local Screen = require("device").screen
-local DEBUG = require("dbg")
 local _ = require("gettext")
 
 local history_dir = "./history/"

frontend/apps/filemanager/filemanagermenu.lua

@@ -18,7 +18,6 @@ local FileManagerMenu = InputContainer:extend{
 }
 
 function FileManagerMenu:init()
-    local filemanager = self.ui
     self.tab_item_table = {
         setting = {
             icon = "resources/icons/appbar.settings.png",
@@ -171,7 +170,7 @@ function FileManagerMenu:onShowMenu()
         dimen = Screen:getSize(),
     }
 
-    local main_menu = nil
+    local main_menu
     if Device:isTouchDevice() then
         local TouchMenu = require("ui/widget/touchmenu")
         main_menu = TouchMenu:new{

frontend/apps/filemanager/filemanagersearch.lua

@@ -302,7 +302,7 @@ function Search:find(option)
         self.data[i] = {"-","-","-","-","-","-","-","-","-"}
 
         if self.use_own_metadata_file then
-            g = io.open(koreaderfile,"r")
+            local g = io.open(koreaderfile, "r")
             line = g:read()
             if line ~= "#metadata.Koreader Version 1.1" then
                 self.use_own_metadata_file =  false

frontend/apps/reader/modules/readerbookmark.lua

@@ -263,11 +263,8 @@ function ReaderBookmark:getDogearBookmarkIndex(pn_or_xp)
 end
 
 function ReaderBookmark:isBookmarkSame(item1, item2)
-    if item1.notes ~= item2.notes then return end
+    if item1.notes ~= item2.notes then return false end
     if self.ui.document.info.has_pages then
-        if not item2.pos0 or not item2.pos1 then return end
-        local zoom1 = item1.pos0.zoom
-        local zoom1 = item2.pos0.zoom
         return item2.pos0 and item2.pos1 and item1.pos0.page == item2.pos0.page
         and item1.pos0.x == item2.pos0.x and item1.pos0.y == item2.pos0.y
         and item1.pos1.x == item2.pos1.x and item1.pos1.y == item2.pos1.y
@@ -281,7 +278,6 @@ end
 function ReaderBookmark:addBookmark(item)
     local _start, _middle, _end, direction = 1, 1, #self.bookmarks, 0
     while _start <= _end do
-        local v = self.bookmarks[_middle]
         _middle = math.floor((_start + _end)/2)
         -- won't add duplicated bookmarks
         if self:isBookmarkSame(item, self.bookmarks[_middle]) then
@@ -301,7 +297,6 @@ end
 function ReaderBookmark:isBookmarkAdded(item)
     local _start, _middle, _end, direction = 1, 1, #self.bookmarks, 0
     while _start <= _end do
-        local v = self.bookmarks[_middle]
         _middle = math.floor((_start + _end)/2)
         if self:isBookmarkSame(item, self.bookmarks[_middle]) then
             return true
@@ -312,6 +307,7 @@ function ReaderBookmark:isBookmarkAdded(item)
             _start, direction = _middle + 1, 1
         end
     end
+    return false
 end
 
 -- binary search to remove bookmark

frontend/apps/reader/modules/readerdictionary.lua

@@ -2,7 +2,6 @@ local InputContainer = require("ui/widget/container/inputcontainer")
 local DictQuickLookup = require("ui/widget/dictquicklookup")
 local InfoMessage = require("ui/widget/infomessage")
 local UIManager = require("ui/uimanager")
-local Geom = require("ui/geometry")
 local Screen = require("device").screen
 local Device = require("device")
 local JSON = require("json")

frontend/apps/reader/modules/readerfont.lua

@@ -8,7 +8,6 @@ local Screen = require("device").screen
 local Input = require("device").input
 local Event = require("ui/event")
 local UIManager = require("ui/uimanager")
-local Screen = require("device").screen
 local DEBUG = require("dbg")
 local T = require("ffi/util").template
 local _ = require("gettext")

frontend/apps/reader/modules/readerfrontlight.lua

@@ -3,7 +3,6 @@ local InputDialog = require("ui/widget/inputdialog")
 local Notification = require("ui/widget/notification")
 local GestureRange = require("ui/gesturerange")
 local UIManager = require("ui/uimanager")
-local Geom = require("ui/geometry")
 local Screen = require("device").screen
 local Device = require("device")
 local DEBUG = require("dbg")

frontend/apps/reader/modules/readermenu.lua

@@ -1,8 +1,5 @@
 local InputContainer = require("ui/widget/container/inputcontainer")
 local CenterContainer = require("ui/widget/container/centercontainer")
-local InfoMessage = require("ui/widget/infomessage")
-local InputDialog = require("ui/widget/inputdialog")
-local ConfirmBox = require("ui/widget/confirmbox")
 local GestureRange = require("ui/gesturerange")
 local UIManager = require("ui/uimanager")
 local Device = require("device")
@@ -158,7 +155,7 @@ function ReaderMenu:onShowReaderMenu()
         dimen = Screen:getSize(),
     }
 
-    local main_menu = nil
+    local main_menu
     if Device:isTouchDevice() then
         local TouchMenu = require("ui/widget/touchmenu")
         main_menu = TouchMenu:new{

frontend/document/credocument.lua

@@ -48,7 +48,7 @@ function CreDocument:cacheInit()
 end
 
 function CreDocument:engineInit()
-    if not engine_initilized then
+    if not self.engine_initilized then
         -- initialize cache
         self:cacheInit()
 
@@ -66,7 +66,7 @@ function CreDocument:engineInit()
             end
         end
 
-        engine_initilized = true
+        self.engine_initilized = true
     end
 end
 

frontend/document/djvudocument.lua

@@ -18,7 +18,7 @@ local DjvuDocument = Document:new{
 
 -- check DjVu magic string to validate
 local function validDjvuFile(filename)
-    f = io.open(filename, "r")
+    local f = io.open(filename, "r")
     if not f then return false end
     local magic = f:read(8)
     f:close()

frontend/ui/language.lua

@@ -4,7 +4,7 @@ local UIManager = require("ui/uimanager")
 local InfoMessage = require("ui/widget/infomessage")
 local _ = require("gettext")
 
-Language = {}
+local Language = {}
 
 function Language:changeLanguage(lang_locale)
     _.changeLang(lang_locale)

frontend/ui/message/streammessagequeue.lua

@@ -1,10 +1,9 @@
 local ffi = require("ffi")
 local DEBUG = require("dbg")
-local util = require("ffi/util")
 local Event = require("ui/event")
 local MessageQueue = require("ui/message/messagequeue")
 
-local dummy = require("ffi/zeromq_h")
+local _ = require("ffi/zeromq_h")
 local zmq = ffi.load("libs/libzmq.so.4")
 local czmq = ffi.load("libs/libczmq.so.1")
 
@@ -25,7 +24,7 @@ function StreamMessageQueue:start()
     end
     local id_size = ffi.new("size_t[1]", 256)
     local buffer = ffi.new("uint8_t[?]", id_size[0])
-    local rc = zmq.zmq_getsockopt(self.socket, ffi.C.ZMQ_IDENTITY, buffer, id_size)
+    rc = zmq.zmq_getsockopt(self.socket, ffi.C.ZMQ_IDENTITY, buffer, id_size)
     self.id = ffi.string(buffer, id_size[0])
     DEBUG("id", #self.id, self.id)
 end

frontend/ui/opdsparser.lua

@@ -32,7 +32,7 @@ function OPDSParser:createFlatXTable(xlex, currentelement)
     local attribute_count = 0;
 
     -- start reading the thing
-    local txt = nil;
+    local txt
     for event, offset, size in xlex:Lexemes() do
         txt = ffi.string(xlex.buf + offset, size)
 

frontend/ui/rendertext.lua

@@ -1,5 +1,4 @@
 local Font = require("ui/font")
-local Screen = require("device").screen
 local Cache = require("cache")
 local CacheItem = require("cacheitem")
 local BlitBuffer = require("ffi/blitbuffer")

frontend/ui/screensaver.lua

@@ -29,7 +29,6 @@ function Screensaver:getCoverImage(file)
                 image_height = image:getHeight()
                 image_width = image:getWidth()
                 local image_ratio = image_width / image_height
-                local screen_ratio = screen_width / screen_height
                 if image_ratio < 1 then
                     image_height = screen_height
                     image_width = image_height * image_ratio

frontend/ui/widget/bboxwidget.lua

@@ -5,7 +5,6 @@ local UIManager = require("ui/uimanager")
 local Device = require("device")
 local GestureRange = require("ui/gesturerange")
 local Math = require("optmath")
-local DEBUG = require("dbg")
 
 --[[
 BBoxWidget shows a bbox for page cropping
@@ -20,7 +19,6 @@ local BBoxWidget = InputContainer:new{
 
 function BBoxWidget:init()
     self.page_bbox = self.document:getPageBBox(self.view.state.page)
-    --DEBUG("used page bbox on page", self.view.state.page, self.page_bbox)
     if Device:isTouchDevice() then
         self.ges_events = {
             TapAdjust = {
@@ -77,7 +75,6 @@ function BBoxWidget:getScreenBBox(page_bbox)
     local bbox = {}
     local scale = self.view.state.zoom
     local screen_offset = self.view.state.offset
-    --DEBUG("screen offset in page_to_screen", screen_offset)
     bbox.x0 = Math.round(page_bbox.x0 * scale + screen_offset.x)
     bbox.y0 = Math.round(page_bbox.y0 * scale + screen_offset.y)
     bbox.x1 = Math.round(page_bbox.x1 * scale + screen_offset.x)
@@ -90,7 +87,6 @@ function BBoxWidget:getPageBBox(screen_bbox)
     local bbox = {}
     local scale = self.view.state.zoom
     local screen_offset = self.view.state.offset
-    --DEBUG("screen offset in screen_to_page", screen_offset)
     bbox.x0 = Math.round((screen_bbox.x0 - screen_offset.x) / scale)
     bbox.y0 = Math.round((screen_bbox.y0 - screen_offset.y) / scale)
     bbox.x1 = Math.round((screen_bbox.x1 - screen_offset.x) / scale)
@@ -106,7 +102,6 @@ function BBoxWidget:inPageArea(ges)
 end
 
 function BBoxWidget:adjustScreenBBox(ges, relative)
-    --DEBUG("adjusting crop bbox with pos", ges.pos)
     if not self:inPageArea(ges) then return end
     local bbox = self.screen_bbox
     local upper_left = Geom:new{ x = bbox.x0, y = bbox.y0}
@@ -125,7 +120,6 @@ function BBoxWidget:adjustScreenBBox(ges, relative)
     local _, nearest = Math.tmin(anchors, function(a,b)
         return a:distance(ges.pos) > b:distance(ges.pos)
     end)
-    --DEBUG("nearest anchor", nearest)
     if nearest == upper_left then
         upper_left.x = ges.pos.x
         upper_left.y = ges.pos.y

frontend/ui/widget/buttondialog.lua

@@ -5,7 +5,6 @@ local CenterContainer = require("ui/widget/container/centercontainer")
 local Device = require("device")
 local GestureRange = require("ui/gesturerange")
 local Geom = require("ui/geometry")
-local Input = require("device").input
 local Screen = require("device").screen
 local UIManager = require("ui/uimanager")
 local _ = require("gettext")

frontend/ui/widget/closebutton.lua

@@ -1,10 +1,7 @@
 local InputContainer = require("ui/widget/container/inputcontainer")
 local FrameContainer = require("ui/widget/container/framecontainer")
-local CenterContainer = require("ui/widget/container/centercontainer")
 local TextWidget = require("ui/widget/textwidget")
 local GestureRange = require("ui/gesturerange")
-local UIManager = require("ui/uimanager")
-local Geom = require("ui/geometry")
 local Font = require("ui/font")
 
 --[[
@@ -25,7 +22,7 @@ function CloseButton:init()
         padding = 0,
         text_widget
     }
-    
+
     self.dimen = text_widget:getSize():copy()
 
     self.ges_events.Close = {

frontend/ui/widget/virtualkeyboard.lua

@@ -47,7 +47,7 @@ function VirtualKey:init()
         self.callback = function () self.keyboard:addChar(self.key) end
     end
 
-    local label_widget = nil
+    local label_widget
     if self.icon then
         label_widget = ImageWidget:new{
             file = self.icon,
@@ -289,7 +289,6 @@ function VirtualKeyboard:addKeys()
         end
     end
 
-    local size = vertical_group:getSize()
     local keyboard_frame = FrameContainer:new{
         margin = 0,
         bordersize = self.bordersize,

spec/unit/readerbookmark_spec.lua

@@ -9,7 +9,7 @@ local DEBUG = require("dbg")
 local sample_epub = "spec/front/unit/data/juliet.epub"
 local sample_pdf = "spec/front/unit/data/sample.pdf"
 
-describe("Readerbookmark module", function()
+describe("ReaderBookmark module", function()
     local function highlight_text(readerui, pos0, pos1)
         readerui.highlight:onHold(nil, { pos = pos0 })
         readerui.highlight:onHoldPan(nil, { pos = pos1 })
@@ -36,6 +36,7 @@ describe("Readerbookmark module", function()
         end)
         UIManager:run()
     end
+
     describe("bookmark for EPUB document", function()
         local page = 10
         local readerui
@@ -49,6 +50,17 @@ describe("Readerbookmark module", function()
             UIManager:show(readerui)
             readerui.rolling:gotoPage(10)
         end)
+        it("should does bookmark comparison properly", function()
+            assert.truthy(readerui.bookmark:isBookmarkSame(
+                { notes = 'foo', page = 1, pos0 = 0, pos1 = 2, },
+                { notes = 'foo', page = 1, pos0 = 0, pos1 = 2, }))
+            assert.falsy(readerui.bookmark:isBookmarkSame(
+                { notes = 'foo', page = 1, pos0 = 0, pos1 = 2, },
+                { notes = 'bar', page = 1, pos0 = 0, pos1 = 2, }))
+            assert.falsy(readerui.bookmark:isBookmarkSame(
+                { notes = 'foo0', page = 1, pos0 = 0, pos1 = 0, },
+                { notes = 'foo', page = 1, pos0 = 0, pos1 = 2, }))
+        end)
         it("should show dogear after togglering non-bookmarked page", function()
             toggler_dogear(readerui)
             Screen:shot("screenshots/reader_bookmark_dogear_epub.png")
@@ -111,6 +123,34 @@ describe("Readerbookmark module", function()
             UIManager:show(readerui)
             readerui.paging:gotoPage(10)
         end)
+        it("should does bookmark comparison properly", function()
+            assert.truthy(readerui.bookmark:isBookmarkSame(
+                { notes = 'foo', pos0 = { page = 1 , x = 2, y = 3},
+                  pos1 = { page = 1, x = 20, y = 3 }, },
+                { notes = 'foo', pos0 = { page = 1 , x = 2, y = 3},
+                  pos1 = { page = 1, x = 20, y = 3 }, }))
+            assert.falsy(readerui.bookmark:isBookmarkSame(
+                { notes = 'foo', page = 1, pos0 = 0, pos1 = 2, },
+                { notes = 'foo', page = 1, pos1 = 2, }))
+            assert.falsy(readerui.bookmark:isBookmarkSame(
+                { notes = 'foo', page = 1, pos0 = 0, pos1 = 2, },
+                { notes = 'foo', page = 1, pos0 = 2, }))
+            assert.falsy(readerui.bookmark:isBookmarkSame(
+                { notes = 'foo', pos0 = { page = 1 , x = 2, y = 3},
+                  pos1 = { page = 1, x = 20, y = 3 }, },
+                { notes = 'foo', pos0 = { page = 2 , x = 2, y = 3},
+                  pos1 = { page = 1, x = 20, y = 3 }, }))
+            assert.falsy(readerui.bookmark:isBookmarkSame(
+                { notes = 'foo', pos0 = { page = 1 , x = 1, y = 3},
+                  pos1 = { page = 1, x = 20, y = 3 }, },
+                { notes = 'foo', pos0 = { page = 1 , x = 2, y = 3},
+                  pos1 = { page = 1, x = 20, y = 3 }, }))
+            assert.falsy(readerui.bookmark:isBookmarkSame(
+                { notes = 'foo', pos0 = { page = 1 , x = 1, y = 3},
+                  pos1 = { page = 1, x = 20, y = 3 }, },
+                { notes = 'foo', pos0 = { page = 1 , x = 1, y = 3},
+                  pos1 = { page = 1, x = 20, y = 2 }, }))
+        end)
         it("should show dogear after togglering non-bookmarked page", function()
             toggler_dogear(readerui)
             Screen:shot("screenshots/reader_bookmark_dogear_pdf.png")
@@ -156,5 +196,31 @@ describe("Readerbookmark module", function()
         it("should get next bookmark for certain page", function()
             assert.are.same(15, readerui.bookmark:getNextBookmarkedPage(10))
         end)
+        it("should search/add bookmarks properly", function()
+            -- clear bookmarks created by previous tests
+            readerui.bookmark.bookmarks = {}
+            local p1 = { x = 0, y = 0, page = 100 }
+            local bm1 = { notes = 'foo', page = 10,
+                          pos0 = { x = 0, y = 0, page = 100 }, pos1 = p1, }
+            assert.falsy(readerui.bookmark:isBookmarkAdded(bm1))
+            readerui.bookmark:addBookmark(bm1)
+            assert.are.same(readerui.bookmark.bookmarks, {bm1})
+
+            local bm2 = { notes = 'foo', page = 1,
+                          pos0 = { x = 0, y = 0, page = 1 }, pos1 = p1, }
+            assert.falsy(readerui.bookmark:isBookmarkAdded(bm2))
+            readerui.bookmark:addBookmark(bm2)
+            assert.are.same({bm1, bm2}, readerui.bookmark.bookmarks)
+
+            local bm3 = { notes = 'foo', page = 5,
+                          pos0 = { x = 0, y = 0, page = 5 }, pos1 = p1, }
+            assert.falsy(readerui.bookmark:isBookmarkAdded(bm3))
+            readerui.bookmark:addBookmark(bm3)
+            assert.are.same({bm1, bm3, bm2}, readerui.bookmark.bookmarks)
+
+            assert.truthy(readerui.bookmark:isBookmarkAdded(bm1))
+            assert.truthy(readerui.bookmark:isBookmarkAdded(bm2))
+            assert.truthy(readerui.bookmark:isBookmarkAdded(bm3))
+        end)
     end)
 end)