Re: [pull] require C99, add robustness and small fixes

[Sami Kerola <kerolasa@xxxxxx>]

On Mon, Jun 11, 2012 at 11:48 AM, Karel Zak <kzak@xxxxxxxxxx> wrote:

New try available in the git repository at:

  git://github.com/kerolasa/lelux-utiliteetit.git 2012wk23

for you to fetch changes up to 1edd8af70b4794c796d0faed0cf763ff3474b92e:

  lsblk: use blkdev_scsi_type_to_name() (2012-06-11 22:06:06 +0200)

----------------------------------------------------------------
Sami Kerola (13):
      setarch: do not use -1 as array index [cppcheck]
      blkid: fix realloc memory leak [cppcheck]
      uuidd: use output redirection which works [checkbashisms]
      include: fix spurious list.h warnings
      build: fix redundant redeclaration warnings
      build: fix unused parameter warnings
      sysfs: fix printf format warnings
      include: fix void pointer arithmetics warnings
      docs: clean up partx.8 manual
      eject: inform if CD-ROM drive is not ready
      wipefs: use symbolic value for markup mode
      blkdev: add blkdev_scsi_type_to_name()
      lsblk: use blkdev_scsi_type_to_name()

> On Sun, Jun 10, 2012 at 11:11:34PM +0200, Sami Kerola wrote:
>> diff --git a/configure.ac b/configure.ac
>> index 2280e7e..13c9637 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -80,6 +80,24 @@ AC_CANONICAL_HOST
>>  AC_C_CONST
>>  AC_C_VOLATILE
>>  AC_C_BIGENDIAN
>> +if test "$cross_compiling" = no; then
>> +  if test "x$ac_cv_prog_cc_c99" = "xno" || test "x$ac_cv_prog_cc_c99"
>> = "x"; then
>> +  # We might be on RHEL5 with a git checkout and so broken
>> +  # autoconf. Check if CC is gcc and if it bails when given -std=gnu99.
>> +  # If not, use that.  Yuck.
>> +    if test "x$ac_cv_c_compiler_gnu" = "xyes"; then
>> +       CC="$CC -std=gnu99"
>> +       AC_RUN_IFELSE(
>> +          [AC_LANG_PROGRAM([],[[
>> +  return 0;
>> +      ]])],
>> +      [],
>> +      [AC_MSG_ERROR([Could not find a C99 compatible compiler])])
>> +    else
>> +      AC_MSG_ERROR([Could not find a C99 compatible compiler])
>> +    fi
>> +  fi
>> +fi
>
>  AC_PROG_CC_C99 sounds really like a better solution, although I have
>  autoconf 2.68 on Fedora 16 :-(

I will leave the C99 update to someone who has better insight how not
to shoot oneself to foot. In other words dropped from the most recent
pull request.

>> --- a/misc-utils/blkid.c
>> +++ b/misc-utils/blkid.c
>> @@ -365,8 +365,10 @@ static int append_str(char **res, size_t *sz,
>> const char *a, const char *b)
>>               return -1;
>>
>>       str = realloc(str, len + 1);
>> -     if (!str)
>> +     if (!str) {
>> +             free(res);
>
>        free(*res);

This is fixed.

>> diff --git a/misc-utils/lsblk.c b/misc-utils/lsblk.c
>> index 9e894ef..f9e05b6 100644
>> --- a/misc-utils/lsblk.c
>> +++ b/misc-utils/lsblk.c
>> @@ -527,6 +527,12 @@ static char *get_type(struct blkdev_cxt *cxt)
>>                               type = "mo-disk"; break;
>>                       case 0x0e: /* TYPE_RBC */
>>                               type = "rbc"; break;
>> +                     default:
>> +                             /* Perhaps include/scsi/scsi.h in
>> +                              * kernel git section 'DEVICE TYPES'
>> +                              * will know what was found.  */
>> +                             sprintf(type, "0x%02x", x);
>> +                             break;
>
>  What about to add scsi_type_to_name() to lib/blkdev.c and add
>  SCSI_TYPE_* macros (TYPE_* sounds too generic) to include/blkdev.h
>  with fallbacks for too old /usr/include/scsi/scsi.h files.

I like this. Have a look of the second last patch which will add
function to blkdev. Perhaps it reassembles to some extent what you
where thinking of.

>> +static void set_exclusive(int *what, int how)
>> +{
>> +     if (*what == ACT_NONE) {
>> +             *what = how;
>> +             return;
>> +     }
>> +     if (*what == how)
>> +             return;
>> +     errx_mutually_exclusive("--{add,delete,show,list,raw,pairs}");
>> +}
>
>  This is good idea, I guess we have the same problem on more places
>  where is the "mutually exclusive" error message.

With quick search I found the following files having some sort of
option exclusiveness going on.  I do not like to add that many changes
to a branch which already has +10 patches, so I dropped the patch for
now. The fix will be included to branch for the ongoing week, which
hopefully has unified way to deal all of the exclusive options.

hwclock/hwclock.c
misc-utils/blkid.c
misc-utils/findmnt.c
misc-utils/lsblk.c
partx/partx.c
sys-utils/losetup.c
sys-utils/lscpu.c
sys-utils/mount.c
sys-utils/prlimit.c
sys-utils/wdctl.c

-- 
   Sami Kerola
   http://www.iki.fi/kerolasa/
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html