Switch to using FAT classes for advanced settings

- No longer relies on operating system for mounting FAT partition
  when applying 'advanced settings'
- change '__attribute__ ((packed))' to '#pragma pack()' as the
  mingw version we are using for Windows has a bug with the former

debian/changelog

@@ -1,3 +1,15 @@
+rpi-imager (1.7.4) unstable; urgency=medium
+
+  * Advanced settings: fix escaping single quotes
+  * Advanced settings: default to using username of logged-in user
+  * Now uses a different method to edit files on the FAT partition
+    to apply advanced settings. Imager now understands the FAT16/FAT32
+    file system format and can edit files by itself using the raw
+    disk device, without having to rely on the operating system
+    to mount the partition first.
+
+ -- Floris Bos <bos@je-eigen-domein.nl>  Mon, 14 Nov 2022 21:49:27 +0100
+
 rpi-imager (1.7.3) unstable; urgency=medium
 
   * Linux: use GnuTLS instead of OpenSSL for computing SHA256

src/CMakeLists.txt

@@ -12,8 +12,8 @@ OPTION (ENABLE_TELEMETRY "Enable sending telemetry" ON)
 project(rpi-imager LANGUAGES CXX C)
 set(IMAGER_VERSION_MAJOR 1)
 set(IMAGER_VERSION_MINOR 7)
-set(IMAGER_VERSION_STR "${IMAGER_VERSION_MAJOR}.${IMAGER_VERSION_MINOR}.3")
+set(IMAGER_VERSION_STR "${IMAGER_VERSION_MAJOR}.${IMAGER_VERSION_MINOR}.4")
-set(IMAGER_VERSION_CSV "${IMAGER_VERSION_MAJOR},${IMAGER_VERSION_MINOR},3,0")
+set(IMAGER_VERSION_CSV "${IMAGER_VERSION_MAJOR},${IMAGER_VERSION_MINOR},4,0")
 add_definitions(-DIMAGER_VERSION_STR="${IMAGER_VERSION_STR}")
 add_definitions(-DIMAGER_VERSION_CSV=${IMAGER_VERSION_CSV})
 

src/devicewrapperfatpartition.cpp

@@ -483,7 +483,7 @@ bool DeviceWrapperFatPartition::dirNameExists(const QByteArray dirname)
     while (readDir(&entry))
     {
         if (!(entry.DIR_Attr & ATTR_LONG_NAME)
-                && dirname == QByteArray(entry.DIR_Name, sizeof(entry.DIR_Name)))
+                && dirname == QByteArray((char *) entry.DIR_Name, sizeof(entry.DIR_Name)))
         {
             return true;
         }

src/devicewrapperstructs.h

@@ -6,6 +6,10 @@
  * Copyright (C) 2022 Raspberry Pi Ltd
  */
 
+#include <stdint.h>
+
+#pragma pack(push, 1)
+
 /* MBR on-disk structures */
 
 struct mbr_partition_entry {
@@ -15,7 +19,7 @@ struct mbr_partition_entry {
     char end_hsc[3];
     unsigned int starting_sector;
     unsigned int nr_of_sectors;
-} __attribute__ ((packed));
+};
 
 struct mbr_table {
     char bootcode[440];
@@ -23,7 +27,7 @@ struct mbr_table {
     unsigned char flags[2];
     mbr_partition_entry part[4];
     unsigned char signature[2];
-} __attribute__ ((packed));
+};
 
 
 /* File Allocation Table
@@ -55,7 +59,7 @@ struct fat16_bpb {
 
     uint8_t  Zeroes[448];
     uint8_t  Signature[2]; /* 0x55aa */
-} __attribute__ ((packed));
+};
 
 struct fat32_bpb {
     uint8_t  BS_jmpBoot[3];
@@ -89,7 +93,7 @@ struct fat32_bpb {
 
     uint8_t  Zeroes[420];
     uint8_t  Signature[2]; /* 0x55aa */
-} __attribute__ ((packed));
+};
 
 union fat_bpb {
     struct fat16_bpb fat16;
@@ -97,7 +101,7 @@ union fat_bpb {
 };
 
 struct dir_entry {
-    char     DIR_Name[11];
+    unsigned char DIR_Name[11];
     uint8_t  DIR_Attr;
     uint8_t  DIR_NTRes;
     uint8_t  DIR_CrtTimeTenth;
@@ -109,7 +113,7 @@ struct dir_entry {
     uint16_t DIR_WrtDate;
     uint16_t DIR_FstClusLO;
     uint32_t DIR_FileSize;
-} __attribute__ ((packed));
+};
 
 struct longfn_entry {
     uint8_t  LDIR_Ord;
@@ -120,7 +124,7 @@ struct longfn_entry {
     char     LDIR_Name2[12];
     uint16_t LDIR_FstClusLO;
     char     LDIR_Name3[4];
-} __attribute__ ((packed));
+};
 
 #define LAST_LONG_ENTRY 0x40
 
@@ -140,6 +144,8 @@ struct FSInfo {
     uint32_t FSI_Nxt_Free;
     uint8_t  FSI_Reserved2[12];
     uint8_t  FSI_TrailSig[4]; /* 0x00 0x00 0x55 0xAA */
-} __attribute__ ((packed));
+};
+
+#pragma pack(pop)
 
 #endif // DEVICEWRAPPERSTRUCTS_H

src/downloadthread.cpp

@@ -5,6 +5,8 @@
 
 #include "downloadthread.h"
 #include "config.h"
+#include "devicewrapper.h"
+#include "devicewrapperfatpartition.h"
 #include "dependencies/mountutils/src/mountutils.hpp"
 #include "dependencies/drivelist/src/drivelist.hpp"
 #include <fstream>
@@ -724,6 +726,15 @@ void DownloadThread::_writeComplete()
 
     emit finalizing();
 
+    if (!_config.isEmpty() || !_cmdline.isEmpty() || !_firstrun.isEmpty())
+    {
+        if (!_customizeImage())
+        {
+            _closeFiles();
+            return;
+        }
+    }
+
     if (_firstBlock)
     {
         qDebug() << "Writing first block (which we skipped at first)";
@@ -741,6 +752,21 @@ void DownloadThread::_writeComplete()
         _firstBlock = nullptr;
     }
 
+    if (!_file.flush())
+    {
+        DownloadThread::_onDownloadError(tr("Error writing to storage (while flushing)"));
+        _closeFiles();
+        return;
+    }
+
+#ifndef Q_OS_WIN
+    if (::fsync(_file.handle()) != 0) {
+        DownloadThread::_onDownloadError(tr("Error writing to storage (while fsync)"));
+        _closeFiles();
+        return;
+    }
+#endif
+
     _closeFiles();
 
 #ifdef Q_OS_DARWIN
@@ -748,18 +774,9 @@ void DownloadThread::_writeComplete()
     _filename.replace("/dev/rdisk", "/dev/disk");
 #endif
 
-    if (_ejectEnabled && _config.isEmpty() && _cmdline.isEmpty() && _firstrun.isEmpty())
+    if (_ejectEnabled)
         eject_disk(_filename.constData());
 
-    if (!_config.isEmpty() || !_cmdline.isEmpty() || !_firstrun.isEmpty())
-    {
-        if (!_customizeImage())
-            return;
-
-        if (_ejectEnabled)
-            eject_disk(_filename.constData());
-    }
-
     emit success();
 }
 
@@ -865,325 +882,111 @@ void DownloadThread::setImageCustomization(const QByteArray &config, const QByte
 
 bool DownloadThread::_customizeImage()
 {
-    QString folder;
-    std::vector<std::string> mountpoints;
-    QByteArray devlower = _filename.toLower();
-
-    emit preparationStatusUpdate(tr("Waiting for FAT partition to be mounted"));
-
-#ifdef Q_OS_WIN
-    qDebug() << "Running diskpart rescan";
-    QProcess proc;
-    proc.setProcessChannelMode(proc.MergedChannels);
-    proc.start("diskpart");
-    proc.waitForStarted();
-    proc.write("rescan\r\n");
-    proc.closeWriteChannel();
-    proc.waitForFinished();
-    qDebug() << proc.readAll();
-#endif
-
-    /* See if OS auto-mounted the device */
-    for (int tries = 0; tries < 3; tries++)
-    {
-        QThread::sleep(1);
-        auto l = Drivelist::ListStorageDevices();
-        for (const auto& i : l)
-        {
-            if (QByteArray::fromStdString(i.device).toLower() == devlower && i.mountpoints.size())
-            {
-                mountpoints = i.mountpoints;
-                break;
-            }
-        }
-    }
-
-#ifdef Q_OS_WIN
-    if (mountpoints.empty() && !_nr.isEmpty()) {
-        qDebug() << "Windows did not assign drive letter automatically. Ask diskpart to do so manually.";
-        proc.start("diskpart");
-        proc.waitForStarted();
-        proc.write("select disk "+_nr+"\r\n"
-                        "select partition 1\r\n"
-                        "assign\r\n");
-        proc.closeWriteChannel();
-        proc.waitForFinished();
-        qDebug() << proc.readAll();
-
-        auto l = Drivelist::ListStorageDevices();
-        for (auto i : l)
-        {
-            if (QByteArray::fromStdString(i.device).toLower() == devlower && i.mountpoints.size())
-            {
-                mountpoints = i.mountpoints;
-                break;
-            }
-        }
-    }
-#endif
-
-#ifdef Q_OS_LINUX
-    bool manualmount = false;
-
-    if (mountpoints.empty())
-    {
-        /* Manually mount folder */
-        manualmount = true;
-        QByteArray fatpartition = _filename;
-        if (isdigit(fatpartition.at(fatpartition.length()-1)))
-            fatpartition += "p1";
-        else
-            fatpartition += "1";
-
-        if (::access(devlower.constData(), W_OK) != 0)
-        {
-            /* Not running as root, try to outsource mounting to udisks2 */
-    #ifndef QT_NO_DBUS
-            UDisks2Api udisks2;
-            QString mp = udisks2.mountDevice(fatpartition);
-            if (!mp.isEmpty())
-                mountpoints.push_back(mp.toStdString());
-    #endif
-        }
-        else
-        {
-            /* Running as root, attempt running mount directly */
-            QTemporaryDir td;
-            QStringList args;
-            mountpoints.push_back(td.path().toStdString());
-            args << "-t" << "vfat" << fatpartition << td.path();
-
-            if (QProcess::execute("mount", args) != 0)
-            {
-                emit error(tr("Error mounting FAT32 partition"));
-                return false;
-            }
-            td.setAutoRemove(false);
-        }
-    }
-#endif
-
-    if (mountpoints.empty())
-    {
-        //
-        qDebug() << "drive info. searching for:" << devlower;
-        auto l = Drivelist::ListStorageDevices();
-        for (const auto& i : l)
-        {
-            qDebug() << "drive" << QByteArray::fromStdString(i.device).toLower();
-            for (const auto& mp : i.mountpoints) {
-                qDebug() << "mountpoint:" << QByteArray::fromStdString(mp);
-            }
-        }
-        //
-
-        emit error(tr("Operating system did not mount FAT32 partition"));
-        return false;
-    }
-
-    /* Some operating system take longer to complete mounting FAT32
-       wait up to 3 seconds for config.txt file to appear */
-    QString configFilename;
-    bool foundFile = false;
-
-    for (int tries = 0; tries < 3; tries++)
-    {
-        /* Search all mountpoints, as on some systems FAT partition
-           may not be first volume */
-        for (const auto& mp : mountpoints)
-        {
-            folder = QString::fromStdString(mp);
-            if (folder.right(1) == '\\')
-                folder.chop(1);
-            configFilename = folder+"/config.txt";
-
-            if (QFile::exists(configFilename))
-            {
-                foundFile = true;
-                break;
-            }
-        }
-        if (foundFile)
-            break;
-        QThread::sleep(1);
-    }
-
-    if (!foundFile)
-    {
-        emit error(tr("Unable to customize. File '%1' does not exist.").arg(configFilename));
-        return false;
-    }
-
     emit preparationStatusUpdate(tr("Customizing image"));
 
-    if (!_config.isEmpty())
+    try
     {
-        auto configItems = _config.split('\n');
+        DeviceWrapper dw(&_file);
-        configItems.removeAll("");
+        if (_firstBlock)
-        QByteArray config;
-
-        QFile f(configFilename);
-        if (f.open(f.ReadOnly))
         {
-            config = f.readAll();
+            /* Outsource first block handling to DeviceWrapper.
-            f.close();
+               It will still not actually be written out yet,
+               until we call sync(), and then it will
+               save the first 4k sector with MBR for last */
+            dw.pwrite(_firstBlock, _firstBlockSize, 0);
+            _bytesWritten += _firstBlockSize;
+            qFreeAligned(_firstBlock);
+            _firstBlock = nullptr;
+        }
+        DeviceWrapperFatPartition *fat = dw.fatPartition(1);
+
+        if (!_config.isEmpty())
+        {
+            auto configItems = _config.split('\n');
+            configItems.removeAll("");
+            QByteArray config = fat->readFile("config.txt");
+
+            for (const QByteArray& item : qAsConst(configItems))
+            {
+                if (config.contains("#"+item)) {
+                    /* Uncomment existing line */
+                    config.replace("#"+item, item);
+                } else if (config.contains("\n"+item)) {
+                    /* config.txt already contains the line */
+                } else {
+                    /* Append new line to config.txt */
+                    if (config.right(1) != "\n")
+                        config += "\n"+item+"\n";
+                    else
+                        config += item+"\n";
+                }
+            }
+
+            fat->writeFile("config.txt", config);
         }
 
-        for (const QByteArray& item : qAsConst(configItems))
+        if (_initFormat == "auto")
         {
-            if (config.contains("#"+item)) {
+            /* Do an attempt at auto-detecting what customization format a custom
-                /* Uncomment existing line */
+               image provided by the user supports */
-                config.replace("#"+item, item);
+            QByteArray issue = fat->readFile("issue.txt");
-            } else if (config.contains("\n"+item)) {
+
-                /* config.txt already contains the line */
+            if (fat->fileExists("user-data"))
-            } else {
+            {
-                /* Append new line to config.txt */
+                /* If we have user-data file on FAT partition, then it must be cloudinit */
-                if (config.right(1) != "\n")
+                _initFormat = "cloudinit";
-                    config += "\n"+item+"\n";
+                qDebug() << "user-data found on FAT partition. Assuming cloudinit support";
-                else
+            }
-                    config += item+"\n";
+            else if (issue.contains("pi-gen"))
+            {
+                /* If issue.txt mentions pi-gen, and there is no user-data file assume
+                 * it is a RPI OS flavor, and use the old systemd unit firstrun script stuff */
+                _initFormat = "systemd";
+                qDebug() << "using firstrun script invoked by systemd customization method";
+            }
+            else
+            {
+                /* Fallback to writing cloudinit file, as it does not hurt having one
+                 * Will just have no customization if OS does not support it */
+                _initFormat = "cloudinit";
+                qDebug() << "Unknown what customization method image supports. Falling back to cloudinit";
             }
         }
 
-        if (f.open(f.WriteOnly) && f.write(config) == config.length())
+        if (!_firstrun.isEmpty() && _initFormat == "systemd")
         {
-            f.close();
+            fat->writeFile("firstrun.sh", _firstrun);
+            _cmdline += " systemd.run=/boot/firstrun.sh systemd.run_success_action=reboot systemd.unit=kernel-command-line.target";
         }
-        else
+
+        if (!_cloudinit.isEmpty() && _initFormat == "cloudinit")
         {
-            emit error(tr("Error writing to config.txt on FAT partition"));
+            _cloudinit = "#cloud-config\n"+_cloudinit;
-            return false;
+            fat->writeFile("user-data", _cloudinit);
         }
+
+        if (!_cloudinitNetwork.isEmpty() && _initFormat == "cloudinit")
+        {
+            fat->writeFile("network-config", _cloudinitNetwork);
+        }
+
+        if (!_cmdline.isEmpty())
+        {
+            QByteArray cmdline = fat->readFile("cmdline.txt").trimmed();
+
+            cmdline += _cmdline;
+
+            fat->writeFile("cmdline.txt", cmdline);
+        }
+        dw.sync();
     }
-
+    catch (std::runtime_error &err)
-    if (_initFormat == "auto")
     {
-        /* Do an attempt at auto-detecting what customization format a custom
+        emit error(err.what());
-           image provided by the user supports */
+        return false;
-        QByteArray issue;
-        QFile fi(folder+"/issue.txt");
-        if (fi.exists() && fi.open(fi.ReadOnly))
-        {
-            issue = fi.readAll();
-            fi.close();
-        }
-
-        if (QFile::exists(folder+"/user-data"))
-        {
-            /* If we have user-data file on FAT partition, then it must be cloudinit */
-            _initFormat = "cloudinit";
-            qDebug() << "user-data found on FAT partition. Assuming cloudinit support";
-        }
-        else if (issue.contains("pi-gen"))
-        {
-            /* If issue.txt mentions pi-gen, and there is no user-data file assume
-             * it is a RPI OS flavor, and use the old systemd unit firstrun script stuff */
-            _initFormat = "systemd";
-            qDebug() << "using firstrun script invoked by systemd customization method";
-        }
-        else
-        {
-            /* Fallback to writing cloudinit file, as it does not hurt having one
-             * Will just have no customization if OS does not support it */
-            _initFormat = "cloudinit";
-            qDebug() << "Unknown what customization method image supports. Falling back to cloudinit";
-        }
-    }
-
-    if (!_firstrun.isEmpty() && _initFormat == "systemd")
-    {
-        QFile f(folder+"/firstrun.sh");
-        if (f.open(f.WriteOnly) && f.write(_firstrun) == _firstrun.length())
-        {
-            f.close();
-        }
-        else
-        {
-            emit error(tr("Error creating firstrun.sh on FAT partition"));
-            return false;
-        }
-
-        _cmdline += " systemd.run=/boot/firstrun.sh systemd.run_success_action=reboot systemd.unit=kernel-command-line.target";
-    }
-
-    if (!_cloudinit.isEmpty() && _initFormat == "cloudinit")
-    {
-        _cloudinit = "#cloud-config\n"+_cloudinit;
-        QFile f(folder+"/user-data");
-        if (f.open(f.WriteOnly) && f.write(_cloudinit) == _cloudinit.length())
-        {
-            f.close();
-        }
-        else
-        {
-            emit error(tr("Error creating user-data cloudinit file on FAT partition"));
-            return false;
-        }
-    }
-
-    if (!_cloudinitNetwork.isEmpty() && _initFormat == "cloudinit")
-    {
-        QFile f(folder+"/network-config");
-        if (f.open(f.WriteOnly) && f.write(_cloudinitNetwork) == _cloudinitNetwork.length())
-        {
-            f.close();
-        }
-        else
-        {
-            emit error(tr("Error creating network-config cloudinit file on FAT partition"));
-            return false;
-        }
-    }
-
-    if (!_cmdline.isEmpty())
-    {
-        QByteArray cmdline;
-
-        QFile f(folder+"/cmdline.txt");
-        if (f.exists() && f.open(f.ReadOnly))
-        {
-            cmdline = f.readAll().trimmed();
-            f.close();
-        }
-
-        cmdline += _cmdline;
-        if (f.open(f.WriteOnly) && f.write(cmdline) == cmdline.length())
-        {
-            f.close();
-        }
-        else
-        {
-            emit error(tr("Error writing to cmdline.txt on FAT partition"));
-            return false;
-        }
     }
 
     emit finalizing();
 
-#ifdef Q_OS_LINUX
-    if (manualmount)
-    {
-        if (::access(devlower.constData(), W_OK) != 0)
-        {
-    #ifndef QT_NO_DBUS
-            UDisks2Api udisks2;
-            udisks2.unmountDrive(devlower);
-    #endif
-        }
-        else
-        {
-            QStringList args;
-            args << folder;
-            QProcess::execute("umount", args);
-            QDir d;
-            d.rmdir(folder);
-        }
-    }
-#endif
-
-#ifndef Q_OS_WIN
-    ::sync();
-#endif
-
     return true;
 }