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

Add package tiny-vcdiff #17797

Merged
merged 4 commits into from
Aug 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
/pkg/openthread/ @Hyungsin @jia200x
/pkg/semtech-loramac/ @aabadie @jia200x
/pkg/tinydtls/ @rfuentess @leandrolanzieri
/pkg/tinyvcdiff/ @jue89
/pkg/u8g2/ @basilfx
/pkg/ucglib/ @basilfx
/pkg/wakaama/ @leandrolanzieri
Expand Down
1 change: 1 addition & 0 deletions pkg/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ rsource "tiny-asn1/Kconfig"
rsource "tinycbor/Kconfig"
rsource "tinycrypt/Kconfig"
rsource "tinydtls/Kconfig"
rsource "tinyvcdiff/Kconfig"
rsource "tlsf/Kconfig"
rsource "tweetnacl/Kconfig"
rsource "u8g2/Kconfig"
Expand Down
46 changes: 46 additions & 0 deletions pkg/tinyvcdiff/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Copyright (c) 2022 Juergen Fitschen
#
# This file is subject to the terms and conditions of the GNU Lesser
# General Public License v2.1. See the file LICENSE in the top level
# directory for more details.

menuconfig PACKAGE_TINYVCDIFF
bool "Tiny VCDIFF"
depends on TEST_KCONFIG

if PACKAGE_TINYVCDIFF

config TINYVCDIFF_BUFFER_SIZE
int "Buffer size"
default 128
help
For VCDIFF copy and run instruction the library requires a buffer.
The best performance is achieved for sizes of typical page sizes of
the underlying MTD or VFS backend. But a size of just 1 byte would
work, too.

menuconfig MODULE_TINYVCDIFF_MTD
bool "MTD Backend"
depends on MODULE_MTD
default y
help
Use a MTD device as VCDIFF target or source.

if MODULE_TINYVCDIFF_MTD

config TINYVCDIFF_MTD_WRITE_SIZE
int "Write size"
default 4
help
Alignment and minimum size for MTD write access.

endif # MODULE_TINYVCDIFF_MTD

config MODULE_TINYVCDIFF_VFS
bool "VFS Backend"
depends on MODULE_VFS
default y
help
Use a VFS file as VCDIFF target or source.

endif # PACKAGE_TINYVCDIFF
9 changes: 9 additions & 0 deletions pkg/tinyvcdiff/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
PKG_NAME=tinyvcdiff
PKG_URL=https://github.com/jue89/tiny-vcdiff.git
PKG_VERSION=e1a679f36d0212f2b84057a70c72f6e9deda45b3
PKG_LICENSE=MIT

include $(RIOTBASE)/pkg/pkg.mk

all:
$(QQ)"$(MAKE)" -C $(PKG_SOURCE_DIR)/src -f $(CURDIR)/$(PKG_NAME).mk
7 changes: 7 additions & 0 deletions pkg/tinyvcdiff/Makefile.dep
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
ifneq (,$(filter mtd,$(USEMODULE)))
USEMODULE += tinyvcdiff_mtd
endif

ifneq (,$(filter vfs,$(USEMODULE)))
USEMODULE += tinyvcdiff_vfs
endif
19 changes: 19 additions & 0 deletions pkg/tinyvcdiff/Makefile.include
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Buffer size for vcdiff
CONFIG_TINYVCDIFF_BUFFER_SIZE ?= 128
CFLAGS += -DVCDIFF_BUFFER_SIZE=$(CONFIG_TINYVCDIFF_BUFFER_SIZE)

# Disable debugging if DEVELHELP is turned off
ifneq ($(DEVELHELP),1)
CFLAGS += -DVCDIFF_NDEBUG
endif

INCLUDES += -I$(PKGDIRBASE)/tinyvcdiff/include
INCLUDES += -I$(RIOTPKG)/tinyvcdiff/include

ifneq (,$(filter tinyvcdiff_mtd,$(USEMODULE)))
DIRS += $(RIOTPKG)/tinyvcdiff/contrib/tinyvcdiff_mtd
endif

ifneq (,$(filter tinyvcdiff_vfs,$(USEMODULE)))
DIRS += $(RIOTPKG)/tinyvcdiff/contrib/tinyvcdiff_vfs
endif
3 changes: 3 additions & 0 deletions pkg/tinyvcdiff/contrib/tinyvcdiff_mtd/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
MODULE = tinyvcdiff_mtd

include $(RIOTBASE)/Makefile.base
179 changes: 179 additions & 0 deletions pkg/tinyvcdiff/contrib/tinyvcdiff_mtd/mtd.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
/*
* Copyright (C) 2022 Juergen Fitschen
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

#include "vcdiff_mtd.h"
jue89 marked this conversation as resolved.
Show resolved Hide resolved
#include "assert.h"
#include <string.h>

#define ENABLE_DEBUG 0
#include "debug.h"

static int _erase (void *dev, size_t offset, size_t len)
{
int rc;
vcdiff_mtd_t *mtd = dev;
size_t addr = offset + len;
size_t page = addr / mtd->dev->page_size;
size_t sector = page / mtd->dev->pages_per_sector;
size_t cnt = sector - mtd->next_erase_sector + 1;

/* early exit if all sectors are already in the erased state */
if (cnt == 0) {
return 0;
}

/* erase sectors */
rc = mtd_erase_sector(mtd->dev, mtd->next_erase_sector, cnt);
if (rc != 0) {
return rc;
}

mtd->next_erase_sector += cnt;

return 0;
}

static int _read (void *dev, uint8_t *dest, size_t offset, size_t len)
{
vcdiff_mtd_t *mtd = dev;

/* mtd_read_page will take care of calculation the right page */
int rc = mtd_read_page(mtd->dev, dest, 0, offset, len);

return rc;
}

static int _align_write (vcdiff_mtd_t *mtd, uint8_t **src, size_t *len)
{
size_t alignment_offset;
size_t copy_len;

alignment_offset = mtd->offset % CONFIG_TINYVCDIFF_MTD_WRITE_SIZE;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
alignment_offset = mtd->offset % CONFIG_TINYVCDIFF_MTD_WRITE_SIZE;
alignment_offset = mtd->offset % mtd->write_size;

should now be possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vcdiff_mtd_t requires write_size to be known at compile time to adjust buffer sizes. But at least an assert() would be nice to have.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use mtd->work_area instead?

Copy link
Contributor Author

@jue89 jue89 Aug 30, 2022

Choose a reason for hiding this comment

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

This would add the dependency mtd_write_page. Currently, I'm not using any of the write page features. Adding this module would add unnecessary code just to have the (mtd_write_page-internal) buffer at hand. It feels a little bit hacky to me.

I'd say having assert()s in place that make sure the buffer is aligned and large enough would be a good thing to have in place.

The default of 4 bytes seems to be a size that fits all mtd devices available in RIOT (IIRC) and isn't wasteful in my opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

For mtd_flashpage it can be larger than 4, same for SD cards (but those are usually used with a filesystem)

if (alignment_offset == 0) {
/* we are already aligned */
return 0;
}

/* check how many bytes have to be copied to be aligned */
copy_len = CONFIG_TINYVCDIFF_MTD_WRITE_SIZE - alignment_offset;
if (copy_len > *len) {
copy_len = *len;
}

/* copy unaligned bytes to the write buffer */
memcpy(&mtd->write_buffer[alignment_offset], *src, copy_len);
alignment_offset += copy_len;
mtd->offset += copy_len;
*len -= copy_len;
*src += copy_len;
DEBUG("_align_write: buffered %zuB for alignment\n", copy_len);

if (alignment_offset < CONFIG_TINYVCDIFF_MTD_WRITE_SIZE) {
/* we haven't collected enough bytes, yet */
return 0;
}

assert(alignment_offset == CONFIG_TINYVCDIFF_MTD_WRITE_SIZE);

DEBUG("_align_write: write buffered data\n");

/* write buffer to mtd
* mtd_read_page will take care of calculation the right page
* it's safe to call mtd_read_page_raw: the erase driver already
* prepared the sector for writing */
return mtd_write_page_raw(mtd->dev, mtd->write_buffer, 0,
mtd->offset - alignment_offset,
CONFIG_TINYVCDIFF_MTD_WRITE_SIZE);
}

static int _write (void *dev, uint8_t *src, size_t offset, size_t len)
{
int rc;
size_t to_copy;
vcdiff_mtd_t *mtd = dev;

assert(offset == mtd->offset);

DEBUG("_write: 0x%zx + %zuB\n", mtd->offset, len);

/* align writes */
rc = _align_write(dev, &src, &len);
if (rc < 0) {
/* an error occurred */
return rc;
} else if (len == 0) {
/* no bytes are left */
return 0;
}

assert((mtd->offset % CONFIG_TINYVCDIFF_MTD_WRITE_SIZE) == 0);

/* copy all aligned data */
to_copy = (len / CONFIG_TINYVCDIFF_MTD_WRITE_SIZE) * CONFIG_TINYVCDIFF_MTD_WRITE_SIZE;
rc = mtd_write_page_raw(mtd->dev, src, 0, mtd->offset, to_copy);
if (rc < 0) {
return rc;
}
mtd->offset += to_copy;
len -= to_copy;
src += to_copy;
if (len == 0) {
/* no bytes are left */
return 0;
}

assert(len < CONFIG_TINYVCDIFF_MTD_WRITE_SIZE);

/* copy remaining bytes into write_buffer */
memcpy(mtd->write_buffer, src, len);
mtd->offset += len;
DEBUG("_write: buffered %zuB for alignment\n", len);

return rc;
}

static int _flush (void *dev)
{
vcdiff_mtd_t *mtd = dev;
int rc;
uint8_t buf[CONFIG_TINYVCDIFF_MTD_WRITE_SIZE];
size_t alignment_offset;

alignment_offset = mtd->offset % CONFIG_TINYVCDIFF_MTD_WRITE_SIZE;
if (alignment_offset == 0) {
/* we are already aligned -> no bytes are left in the buffer */
return 0;
}

DEBUG("_flush: write last %zuB\n", alignment_offset);

/* get present bytes from MTD to pad alignment */
rc = mtd_read_page(mtd->dev, buf, 0, mtd->offset - alignment_offset,
CONFIG_TINYVCDIFF_MTD_WRITE_SIZE);
if (rc < 0) {
return rc;
}

/* pad write buffer */
memcpy(&mtd->write_buffer[alignment_offset], &buf[alignment_offset],
CONFIG_TINYVCDIFF_MTD_WRITE_SIZE - alignment_offset);

/* write last buffer */
rc = mtd_write_page_raw(mtd->dev, mtd->write_buffer, 0,
mtd->offset - alignment_offset,
CONFIG_TINYVCDIFF_MTD_WRITE_SIZE);

return rc;
}

const vcdiff_driver_t vcdiff_mtd_driver = {
.erase = _erase,
.read = _read,
.write = _write,
.flush = _flush
};
3 changes: 3 additions & 0 deletions pkg/tinyvcdiff/contrib/tinyvcdiff_vfs/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
MODULE = tinyvcdiff_vfs

include $(RIOTBASE)/Makefile.base
69 changes: 69 additions & 0 deletions pkg/tinyvcdiff/contrib/tinyvcdiff_vfs/vfs.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright (C) 2022 Juergen Fitschen
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

#include "vcdiff_vfs.h"
#include <unistd.h>
#include "errno.h"

static int _read (void *dev, uint8_t *dest, size_t offset, size_t len)
{
int rc;
vcdiff_vfs_t *vfs = dev;

/* seek to the given offset */
rc = vfs_lseek(vfs->fd, offset, SEEK_SET);
if (rc < 0) {
return rc;
} else if ((size_t) rc != offset) {
return -EFAULT;
}

/* read from file into the buffer */
rc = vfs_read(vfs->fd, dest, len);
if (rc < 0) {
return rc;
} else if ((size_t) rc != len) {
return -EFAULT;
}

return 0;
}

static int _write (void *dev, uint8_t *src, size_t offset, size_t len)
{
int rc;
vcdiff_vfs_t *vfs = dev;

/* make sure to stay in the set file size bounds */
if (offset + len > vfs->max_size) {
return -EFBIG;
}

/* seek to the given offset */
rc = vfs_lseek(vfs->fd, offset, SEEK_SET);
if (rc < 0) {
return rc;
} else if ((size_t) rc != offset) {
return -EFAULT;
}

/* write all bytes to the file */
rc = vfs_write(vfs->fd, src, len);
if (rc < 0) {
return rc;
} else if ((size_t) rc != len) {
return -EFAULT;
}

return 0;
}

const vcdiff_driver_t vcdiff_vfs_driver = {
.read = _read,
.write = _write
};
Loading