Hi Balaton, Philippe

On 24/10/25 16:55, BALATON Zoltan wrote:
On Fri, 24 Oct 2025, Vishal Chourasia wrote:
Add error checking for lseek() failure and provide better error
messages when image loading fails, including filenames and addresses.

Signed-off-by: Vishal Chourasia <[email protected]>
---
hw/core/loader.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 7aca4989ef..48dd4e7b33 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -79,6 +79,10 @@ int64_t get_image_size(const char *filename, Error **errp)
    if (fd < 0)
        return -1;
    size = lseek(fd, 0, SEEK_END);
+    if (size < 0) {
+        error_setg_errno(errp, errno, "lseek failure: %s", filename);
+        return -1;
+    }
    close(fd);
    return size;
}
@@ -129,14 +133,24 @@ ssize_t load_image_targphys_as(const char *filename,                                hwaddr addr, uint64_t max_sz, AddressSpace *as,
                               Error **errp)
{
+    ERRP_GUARD();
    ssize_t size;

    size = get_image_size(filename, errp);
-    if (size < 0 || size > max_sz) {
+    if (*errp) {
+        return -1;
+    }
+
+    if (size > max_sz) {
+        error_setg(errp, "%s exceeds maximum image size (%" PRIu64 " MiB)",
+                   filename, max_sz / MiB);

MiB is arbitrary here. This function is used to load all kinds of images such as ROMs which may be 64k-2MB or even executables in generic loader that can be a few kilobytes. This might result in errors saying max size is 0 MiB if the allowed size is less than a MiB (e.g. amigaone PROM_SIZE = 512 KiB) and integer division discards fractions. Do we have a function to pretty print sizes or maybe this should be left as bytes or at most kilobytes?

Yes, for images sizes less than a megabyte.
Perhaps we can leave it at Kilobytes

I will send out another version.
I will also address the *errp vs size<0 comment from Philippe.

Thanks,
vishalc

Regards,
BALATON Zoltan

        return -1;
    }
+
    if (size > 0) {
        if (rom_add_file_fixed_as(filename, addr, -1, as) < 0) {
+            error_setg(errp, "could not load '%s' at %" HWADDR_PRIx,
+                       filename, addr);
            return -1;
        }
    }


Reply via email to