Reduce dependeny of hdparm

[Knogle]

Ahoy, i hope you are doing fine :) As i see, hdparm is not really a library, it's a command line tool. So we are executing command line operations, and parsing the output. Isn't it more appropriate to use ioctl commands instead, utilizing the ioctl.h libary? Just a consideration :)

e.g. Temperature could be read this way.

#include <stdio.h>
#include <fcntl.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <linux/hdreg.h>

int main(int argc, char **argv) {
    int fd, result;
    struct hd_drive_cmd_hdr cmd;
    unsigned char buf[512];

    if (argc < 2) {
        fprintf(stderr, "Usage: %s <device>\n", argv[0]);
        return 1;
    }

    fd = open(argv[1], O_RDONLY | O_NONBLOCK);
    if (fd < 0) {
        perror("Error opening device");
        return 1;
    }

    // Preparing SMART command to read data
    memset(&cmd, 0, sizeof(cmd));
    memset(buf, 0, sizeof(buf));
    cmd.command = WIN_SMART;
    cmd.features = SMART_READ_VALUES;
    cmd.sector_count = 1;
    cmd.sector_number = 1;
    cmd.lbam = SMART_CYL_LOW;
    cmd.lbah = SMART_CYL_HI;

    // The buffer must be set up as an HDIO_DRIVE_CMD ioctl() parameter
    buf[0] = 'C'; // SMART command
    buf[1] = cmd.features;
    buf[2] = cmd.sector_count;
    buf[3] = cmd.sector_number;
    result = ioctl(fd, HDIO_DRIVE_CMD, &buf);

    if (result < 0) {
        perror("Error executing SMART READ DATA command");
        close(fd);
        return 1;
    }

    // Temperature is usually stored in byte 194 of the data structure
    printf("HDD Temperature: %d°C\n", buf[194]);

    close(fd);
    return 0;
}

[PartialVolume]

Isn't it more appropriate to use ioctl commands instead, utilizing the ioctl.h library?

It's always been my preference to use ioctl and we do for some things like retrieval of HPA/DSO data which was necessary due to a bug in certain versions of hdparm, however currently nwipe uses multiple methods to obtain temperature including the kernels hwmon driver and separate code for SAS drives. Adding or replacing hdparm should be done in such a way that doesn't break any other method. It's also not only about obtaining the actual temperature but also the temperature limits of which there maybe be a further four temperature values that need to be retrieved. Depending on the drive not all these values are available. To make any change to temperature you need to understand how temperature limit values are used within the GUI too. It was also be good to understand hdparms code to makesure there aren't edge cases in obtaining temperature that we know nothing about and so end up with less temperatures being reported.

[Knogle]

Can you maybe make a breakdown of which functions are using hdparm and smartmontools in the current code?

[vifino]

Hey! Just chiming in here to mention that I would like to see nwipe no longer shell out whatsoever, specifically to hdparm, smartmontools, dmidecode, and modprobe as the logic around calling them seems very suspicious to me. From a security perspective those aren't too great, either.

[PartialVolume]

as the logic around calling them seems very suspicious to me.

Why do you think it's suspicious? these are all very powerful tools designed to operate with root privileges. Without that level of privilege they would be unable to do the job they are designed to do.

I do agree though in regards to bringing the code into nwipe in the form of low level functions. It makes for a cleaner solution and less dependencies that could break nwipe when their text output changes.

I see you code. we're always open to pull requests should you want to have a go at this. hdparm would be the easiest to remove as I've already written the low level function for accessing HPA/DCO status which can be used as an example.