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

[Rod S. <rod...@ro...>]

On 4/15/22 11:51, bal...@un... wrote:
> 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":

Thanks for reporting this, and finding a fix! I've just committed your
fix to the GPT fdisk git repository.

>   ----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
> 
> 
> _______________________________________________
> Gptfdisk-general mailing list
> Gpt...@li...
> https://lists.sourceforge.net/lists/listinfo/gptfdisk-general


-- 
Rod Smith
rod...@ro...
http://www.rodsbooks.com