[Gptfdisk-general] problem when building vs popt master (7182e4618ad5a0186145fc2aa4a98c2229afdfa8, revno: 740)

[<bal...@un...>]

hello

this is actually more an headsup than a bug report

I just run the build of gptfdisk-1.0.9 (on linux) vs popt master (as per
the subject) and got the following error from sgdisk at "make test":

  ----8<----
  [...]
  **Testing sgdisk binary**

  Problem opening  for reading! Error is 2.
  The specified file does not exist!
  Warning! Unable to generate a proper UUID! Creating an improper one as a last
  resort! Windows 7 may crash if you save this partition table!
  Information: Creating fresh partition table; will override earlier problems!
  Caution! Secondary header was placed beyond the disk's limits! Moving the
  header, but other problems may occur!
  Unable to open device '' for writing! Errno is 2! Aborting write!
  **FAILED** sgdisk return 4 when creating partition table
  make[1]: *** [Makefile:160: test] Error 1
  make[1]: Leaving directory '/home/balducci/tmp/install-us-d/gptfdisk-1.0.9.d/gptfdisk-1.0.9'
  ---->8----

Note that for gdisk all tests complete successfully

After some debugging, I seem to have clarified the situation.
sgdisk (but not gdisk) uses popt to parse command line opts.
In gptcl.cc the device filename is read from the command line  as:
    ----8<----
      device = (char*) poptGetArg(poptCon);
    ---->8----

Note that poptGetArg returns its own allocated memory, not a copy of
it:
    ----8<----
    const char * poptGetArg(poptContext con)
    {
        const char * ret = NULL;
        if (con && con->leftovers != NULL && con->nextLeftover < con->numLeftovers)
            ret = con->leftovers[con->nextLeftover++];
        return ret;
    }
    ---->8----

Immediately after the poptGetArg call, gptcl.cc does:
    ----8<----
      poptResetContext(poptCon);
    ---->8----

Now: in the latest commit to popt
(7182e4618ad5a0186145fc2aa4a98c2229afdfa8, revno: 740) the
poptResetContext function (in popt.c) *FREES* the leftovers (which it
didn't until 740):

    ----8<----
    ##> git diff a4afb898f7758160acda71d774c7d98d528da273 7182e4618ad5a0186145fc2aa4a98c2229afdfa8
    diff --git a/src/popt.c b/src/popt.c
    index 0521c8d..f3f26a3 100644
    --- a/src/popt.c
    +++ b/src/popt.c
    @@ -216,6 +216,9 @@ void poptResetContext(poptContext con)
         else
            con->os->next = 0;

    +    for (i = 0; i < con->numLeftovers; i++) {
    +        con->leftovers[i] = _free(con->leftovers[i]);
    +    }
         con->numLeftovers = 0;
         con->nextLeftover = 0;
         con->restLeftover = 0;
    @@ -1534,7 +1537,7 @@ poptContext poptFreeContext(poptContext con)
         con->numExecs = 0;

         for (i = 0; i < con->numLeftovers; i++) {
    -        con->leftovers[i] = _free(&con->leftovers[i]);
    +        con->leftovers[i] = _free(con->leftovers[i]);
         }
         con->leftovers = _free(con->leftovers);
    ---->8----

The end result is that when sgdisk is linked vs latest popt (revno
740), the device variable in gptcl.cc does not point to valid memory
after "poptResetContext(poptCon);" and causes the error reported above.

Note that this problem didn't exist before popt-740
(7182e4618ad5a0186145fc2aa4a98c2229afdfa8) because for previous popt
revisions the leftovers were not freed in poptResetContext.

As a matter of fact, this fixes everything for me and the tests
complete successfully also for sgdisk:

    ----8<----
     diff -c ./gptcl.cc.USE_STRDUP ./gptcl.cc
    *** ./gptcl.cc.USE_STRDUP       Fri Apr 15 16:33:50 2022
    --- ./gptcl.cc  Fri Apr 15 16:33:50 2022
    ***************
    *** 155,161 ****
         } // while

         // Assume first non-option argument is the device filename....
    !    device = (char*) poptGetArg(poptCon);
         poptResetContext(poptCon);

         if (device != NULL) {
    --- 155,161 ----
         } // while

         // Assume first non-option argument is the device filename....
    !    device = strdup((char*) poptGetArg(poptCon));
         poptResetContext(poptCon);

         if (device != NULL) {
    ---->8----

As I said, this is just to let you know the potential problem when
sgdisk is linked vs popt>=740

I thank you very much for your excellent work

ciao
-gabriele