Debian Bug report logs - #348072
qsort/bsearch should use more robust example code

version graph

Package: manpages-dev; Maintainer for manpages-dev is Martin Schulze <joey@debian.org>; Source for manpages-dev is src:manpages.

Reported by: Falk Hueffner <falk@debian.org>

Date: Sat, 14 Jan 2006 16:03:16 UTC

Severity: normal

Tags: patch

Found in version manpages-dev/2.17-1

Fixed in version manpages/2.76-1

Done: Martin Schulze <joey@infodrom.org>

Bug is archived. No further changes may be made.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to debian-bugs-dist@lists.debian.org, Martin Schulze <joey@debian.org>:
Bug#348072; Package manpages-dev. Full text and rfc822 format available.

Acknowledgement sent to Falk Hueffner <falk@debian.org>:
New Bug report received and forwarded. Copy sent to Martin Schulze <joey@debian.org>. Full text and rfc822 format available.

Message #5 received at submit@bugs.debian.org (full text, mbox):

From: Falk Hueffner <falk@debian.org>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: qsort: wrong claim about strcmp being suitable as "compar" argument
Date: Sat, 14 Jan 2006 16:51:01 +0100
Package: manpages-dev
Version: 2.17-1
Severity: normal
File: /usr/share/man/man3/qsort.3.gz

The man page claims:

NOTE
       Library  routines  suitable for use as the compar argument include str-
       cmp(), alphasort(), and versionsort().

But strcmp is not suitable (see
http://www.lysator.liu.se/c/c-faq/c-12.html#12-2).

	Falk


-- System Information:
Debian Release: testing/unstable
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: alpha
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.13.2
Locale: LANG=C, LC_CTYPE=de_DE@euro (charmap=ISO-8859-15)

Versions of packages manpages-dev depends on:
ii  manpages                      2.17-1     Manual pages about using a GNU/Lin

manpages-dev recommends no packages.

-- no debconf information



Information forwarded to debian-bugs-dist@lists.debian.org, Martin Schulze <joey@debian.org>:
Bug#348072; Package manpages-dev. Full text and rfc822 format available.

Acknowledgement sent to "Michael Kerrisk" <mtk-manpages@gmx.net>:
Extra info received and forwarded to list. Copy sent to Martin Schulze <joey@debian.org>. Full text and rfc822 format available.

Message #10 received at 348072@bugs.debian.org (full text, mbox):

From: "Michael Kerrisk" <mtk-manpages@gmx.net>
To: Falk Hueffner <falk@debian.org>,348072@bugs.debian.org
Cc: control@bugs.debian.org
Subject: Re: Bug#348072: qsort: wrong claim about strcmp being suitable as "compar" argument
Date: Sat, 14 Jan 2006 19:25:44 +0100 (MET)
> The man page claims:
> 
> NOTE
>    Library  routines  suitable for use as the compar argument include
>    strcmp(), alphasort(), and versionsort().
> 
> But strcmp is not suitable (see
> http://www.lysator.liu.se/c/c-faq/c-12.html#12-2).

Hello Falk,

The statement on that page is only true if one includes its 
proviso:

    By "array of strings" you probably mean 
    "array of pointers to char."

If one is talking about an array of (fixed-length) strings,
rather than an array of pointers to strings, then strcmp()
is okay.  But I agree that that is likely the less common 
case, and therefore the reader of the manual page could 
be misled.

I have fixed this in the upstream 2.21 release by including
a small example that demonstrates how strcmp() should
be used (like in the page you refer to).

Thanks for your report.

Cheers,

Michael

-- 
Michael Kerrisk
maintainer of Linux man pages Sections 2, 3, 4, 5, and 7 

Want to help with man page maintenance?  Grab the latest
tarball at
ftp://ftp.win.tue.nl/pub/linux-local/manpages/, read 
the HOWTOHELP file and grep the source files for 'FIXME'.



Information forwarded to debian-bugs-dist@lists.debian.org, Martin Schulze <joey@debian.org>:
Bug#348072; Package manpages-dev. Full text and rfc822 format available.

Acknowledgement sent to Falk Hueffner <falk@debian.org>:
Extra info received and forwarded to list. Copy sent to Martin Schulze <joey@debian.org>. Full text and rfc822 format available.

Message #15 received at 348072@bugs.debian.org (full text, mbox):

From: Falk Hueffner <falk@debian.org>
To: "Michael Kerrisk" <mtk-manpages@gmx.net>
Cc: 348072@bugs.debian.org
Subject: Re: Bug#348072: qsort: wrong claim about strcmp being suitable as "compar" argument
Date: Sat, 14 Jan 2006 19:49:47 +0100
"Michael Kerrisk" <mtk-manpages@gmx.net> writes:

> I have fixed this in the upstream 2.21 release by including a small
> example that demonstrates how strcmp() should be used (like in the
> page you refer to).

Thanks for your quick reply. Just another minor suggestion, the
example code uses:

            qsort(months, nr_of_months, sizeof(struct mi), compmi);
[...]
                 res = bsearch(&key, months, nr_of_months,
                            sizeof(struct mi), compmi);

I think it would be better to use

            qsort(months, nr_of_months, sizeof *months, compmi);
[...]
                 res = bsearch(&key, months, nr_of_months,
                            sizeof *months, compmi);

since this is more robust against changes and easier to verify for a
human reader.

-- 
	Falk



Information forwarded to debian-bugs-dist@lists.debian.org, Martin Schulze <joey@debian.org>:
Bug#348072; Package manpages-dev. Full text and rfc822 format available.

Acknowledgement sent to Falk Hueffner <falk@debian.org>:
Extra info received and forwarded to list. Copy sent to Martin Schulze <joey@debian.org>. Full text and rfc822 format available.

Message #20 received at 348072@bugs.debian.org (full text, mbox):

From: Falk Hueffner <falk@debian.org>
To: 348072@bugs.debian.org
Cc: control@bugs.debian.org
Subject: Bug#348072: qsort/bsearch should use more robust example code
Date: Sun, 01 Oct 2006 13:59:34 +0200
retitle 348072 qsort/bsearch should use more robust example code
tags 348072 + patch
thanks

Here's a patch for the remaining issue.

diff -Nurp manpages-2.39/man3/bsearch.3 manpages-2.39.hacked/man3/bsearch.3
--- manpages-2.39/man3/bsearch.3	2006-08-03 15:57:30.000000000 +0200
+++ manpages-2.39.hacked/man3/bsearch.3	2006-10-01 13:54:59.000000000 +0200
@@ -75,7 +75,7 @@ struct mi {
 	{ 9, "sep" }, {10, "oct" }, {11, "nov" }, {12, "dec" }
 };
 
-#define nr_of_months (sizeof(months)/sizeof(months[0]))
+#define nr_of_months (sizeof months / sizeof *months)
 
 static int compmi(const void *m1, const void *m2) {
 	struct mi *mi1 = (struct mi *) m1;
@@ -86,12 +86,12 @@ static int compmi(const void *m1, const 
 int main(int argc, char **argv) {
 	int i;
 
-	qsort(months, nr_of_months, sizeof(struct mi), compmi);
+	qsort(months, nr_of_months, sizeof *months, compmi);
 	for (i = 1; i < argc; i++) {
 		struct mi key, *res;
 		key.name = argv[i];
 		res = bsearch(&key, months, nr_of_months,
-			      sizeof(struct mi), compmi);
+			      sizeof *months, compmi);
 		if (res == NULL)
 			printf("'%s': unknown month\en", argv[i]);
 		else
diff -Nurp manpages-2.39/man3/qsort.3 manpages-2.39.hacked/man3/qsort.3
--- manpages-2.39/man3/qsort.3	2006-08-03 15:57:30.000000000 +0200
+++ manpages-2.39.hacked/man3/qsort.3	2006-10-01 13:53:16.000000000 +0200
@@ -98,7 +98,7 @@ main(int argc, char *argv[])
 
     assert(argc > 1);
 
-    qsort(&argv[1], argc - 1, sizeof(char *), cmpstringp);
+    qsort(&argv[1], argc - 1, sizeof argv[1], cmpstringp);
 
     for (j = 1; j < argc; j++)
         puts(argv[j]);


-- 
	Falk



Changed Bug title. Request was from Falk Hueffner <falk@debian.org> to control@bugs.debian.org. Full text and rfc822 format available.

Tags added: patch Request was from Falk Hueffner <falk@debian.org> to control@bugs.debian.org. Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, Martin Schulze <joey@debian.org>:
Bug#348072; Package manpages-dev. Full text and rfc822 format available.

Acknowledgement sent to Martin Schulze <joey@infodrom.org>:
Extra info received and forwarded to list. Copy sent to Martin Schulze <joey@debian.org>. Full text and rfc822 format available.

Message #29 received at 348072@bugs.debian.org (full text, mbox):

From: Martin Schulze <joey@infodrom.org>
To: Falk Hueffner <falk@debian.org>, 348072@bugs.debian.org
Subject: Re: Bug#348072: qsort: wrong claim about strcmp being suitable as "compar" argument
Date: Wed, 30 Jan 2008 14:50:14 +0100
Falk Hueffner wrote:
> "Michael Kerrisk" <mtk-manpages@gmx.net> writes:
> 
> > I have fixed this in the upstream 2.21 release by including a small
> > example that demonstrates how strcmp() should be used (like in the
> > page you refer to).
> 
> Thanks for your quick reply. Just another minor suggestion, the
> example code uses:
> 
>             qsort(months, nr_of_months, sizeof(struct mi), compmi);
> [...]
>                  res = bsearch(&key, months, nr_of_months,
>                             sizeof(struct mi), compmi);
> 
> I think it would be better to use
> 
>             qsort(months, nr_of_months, sizeof *months, compmi);
> [...]
>                  res = bsearch(&key, months, nr_of_months,
>                             sizeof *months, compmi);
> 

I don't think so.  From the declaration months is of type
struct mi[].  Thus, the size of each element is sizeof(struct mi),
logically.

Regards,

	Joey

-- 
Life is too short to run proprietary software.  -- Bdale Garbee

Please always Cc to me when replying to me on the lists.




Information forwarded to debian-bugs-dist@lists.debian.org, Martin Schulze <joey@debian.org>:
Bug#348072; Package manpages-dev. Full text and rfc822 format available.

Acknowledgement sent to Martin Schulze <joey@infodrom.org>:
Extra info received and forwarded to list. Copy sent to Martin Schulze <joey@debian.org>. Full text and rfc822 format available.

Message #34 received at 348072@bugs.debian.org (full text, mbox):

From: Martin Schulze <joey@infodrom.org>
To: Falk Hueffner <falk@debian.org>, 348072@bugs.debian.org
Subject: Re: Bug#348072: qsort/bsearch should use more robust example code
Date: Wed, 30 Jan 2008 14:54:03 +0100
Oh, btw. thanks anyway!

Regards,

	Joey

-- 
Life is too short to run proprietary software.  -- Bdale Garbee

Please always Cc to me when replying to me on the lists.




Information forwarded to debian-bugs-dist@lists.debian.org, Martin Schulze <joey@debian.org>:
Bug#348072; Package manpages-dev. Full text and rfc822 format available.

Acknowledgement sent to Martin Schulze <joey@infodrom.org>:
Extra info received and forwarded to list. Copy sent to Martin Schulze <joey@debian.org>. Full text and rfc822 format available.

Message #39 received at 348072@bugs.debian.org (full text, mbox):

From: Martin Schulze <joey@infodrom.org>
To: Falk Hueffner <falk@debian.org>, 348072@bugs.debian.org
Subject: Re: Bug#348072: qsort/bsearch should use more robust example code
Date: Wed, 30 Jan 2008 14:53:42 +0100
Falk Hueffner wrote:
> Here's a patch for the remaining issue.
> 
> diff -Nurp manpages-2.39/man3/bsearch.3 manpages-2.39.hacked/man3/bsearch.3
> --- manpages-2.39/man3/bsearch.3	2006-08-03 15:57:30.000000000 +0200
> +++ manpages-2.39.hacked/man3/bsearch.3	2006-10-01 13:54:59.000000000 +0200
> @@ -75,7 +75,7 @@ struct mi {
>  	{ 9, "sep" }, {10, "oct" }, {11, "nov" }, {12, "dec" }
>  };
>  
> -#define nr_of_months (sizeof(months)/sizeof(months[0]))
> +#define nr_of_months (sizeof months / sizeof *months)

Adjusted to:

Index: man3/bsearch.3
===================================================================
RCS file: /var/cvs/debian/manpages/man3/bsearch.3,v
retrieving revision 1.1.1.25
diff -u -p -r1.1.1.25 bsearch.3
--- man3/bsearch.3	28 Jan 2008 10:08:13 -0000	1.1.1.25
+++ man3/bsearch.3	30 Jan 2008 13:46:03 -0000
@@ -83,7 +83,7 @@ struct mi {
     { 9, "sep" }, {10, "oct" }, {11, "nov" }, {12, "dec" }
 };
 
-#define nr_of_months (sizeof(months)/sizeof(months[0]))
+#define nr_of_months (sizeof(months)/sizeof(struct mi))
 
 static int
 compmi(const void *m1, const void *m2)

> diff -Nurp manpages-2.39/man3/qsort.3 manpages-2.39.hacked/man3/qsort.3
> --- manpages-2.39/man3/qsort.3	2006-08-03 15:57:30.000000000 +0200
> +++ manpages-2.39.hacked/man3/qsort.3	2006-10-01 13:53:16.000000000 +0200
> @@ -98,7 +98,7 @@ main(int argc, char *argv[])
>  
>      assert(argc > 1);
>  
> -    qsort(&argv[1], argc - 1, sizeof(char *), cmpstringp);
> +    qsort(&argv[1], argc - 1, sizeof argv[1], cmpstringp);
>  
>      for (j = 1; j < argc; j++)
>          puts(argv[j]);

Ok, but with brackets for continuity

Index: man3/qsort.3
===================================================================
RCS file: /var/cvs/debian/manpages/man3/qsort.3,v
retrieving revision 1.1.1.28
diff -u -p -r1.1.1.28 qsort.3
--- man3/qsort.3	28 Jan 2008 10:05:42 -0000	1.1.1.28
+++ man3/qsort.3	30 Jan 2008 13:51:04 -0000
@@ -103,7 +103,7 @@ main(int argc, char *argv[])
 
     assert(argc > 1);
 
-    qsort(&argv[1], argc \- 1, sizeof(char *), cmpstringp);
+    qsort(&argv[1], argc \- 1, sizeof(argv[1]), cmpstringp);
 
     for (j = 1; j < argc; j++)
         puts(argv[j]);

-- 
Life is too short to run proprietary software.  -- Bdale Garbee

Please always Cc to me when replying to me on the lists.




Information forwarded to debian-bugs-dist@lists.debian.org, Martin Schulze <joey@debian.org>:
Bug#348072; Package manpages-dev. Full text and rfc822 format available.

Acknowledgement sent to Falk Hueffner <falk@debian.org>:
Extra info received and forwarded to list. Copy sent to Martin Schulze <joey@debian.org>. Full text and rfc822 format available.

Message #44 received at 348072@bugs.debian.org (full text, mbox):

From: Falk Hueffner <falk@debian.org>
To: Martin Schulze <joey@infodrom.org>
Cc: 348072@bugs.debian.org
Subject: Re: Bug#348072: qsort: wrong claim about strcmp being suitable as "compar" argument
Date: Wed, 30 Jan 2008 15:23:41 +0100
Martin Schulze <joey@infodrom.org> writes:

> Falk Hueffner wrote:
>> Just another minor suggestion, the example code uses:
>> 
>>             qsort(months, nr_of_months, sizeof(struct mi), compmi);
>> [...]
>>                  res = bsearch(&key, months, nr_of_months,
>>                             sizeof(struct mi), compmi);
>> 
>> I think it would be better to use
>> 
>>             qsort(months, nr_of_months, sizeof *months, compmi);
>> [...]
>>                  res = bsearch(&key, months, nr_of_months,
>>                             sizeof *months, compmi);
>> 
>
> I don't think so.  From the declaration months is of type
> struct mi[].  Thus, the size of each element is sizeof(struct mi),
> logically.

That's true. However, that code is less robust, since changing the
type of *months now requires to change two places, and verifying the
correctness of the call requires you to look up the declaration.

-- 
	Falk




Information forwarded to debian-bugs-dist@lists.debian.org, Martin Schulze <joey@debian.org>:
Bug#348072; Package manpages-dev. Full text and rfc822 format available.

Acknowledgement sent to Martin Schulze <joey@infodrom.org>:
Extra info received and forwarded to list. Copy sent to Martin Schulze <joey@debian.org>. Full text and rfc822 format available.

Message #49 received at 348072@bugs.debian.org (full text, mbox):

From: Martin Schulze <joey@infodrom.org>
To: Falk Hueffner <falk@debian.org>
Cc: 348072@bugs.debian.org
Subject: Re: Bug#348072: qsort: wrong claim about strcmp being suitable as "compar" argument
Date: Wed, 30 Jan 2008 15:38:15 +0100
Falk Hueffner wrote:
> Martin Schulze <joey@infodrom.org> writes:
> 
> > Falk Hueffner wrote:
> >> Just another minor suggestion, the example code uses:
> >> 
> >>             qsort(months, nr_of_months, sizeof(struct mi), compmi);
> >> [...]
> >>                  res = bsearch(&key, months, nr_of_months,
> >>                             sizeof(struct mi), compmi);
> >> 
> >> I think it would be better to use
> >> 
> >>             qsort(months, nr_of_months, sizeof *months, compmi);
> >> [...]
> >>                  res = bsearch(&key, months, nr_of_months,
> >>                             sizeof *months, compmi);
> >> 
> >
> > I don't think so.  From the declaration months is of type
> > struct mi[].  Thus, the size of each element is sizeof(struct mi),
> > logically.
> 
> That's true. However, that code is less robust, since changing the
> type of *months now requires to change two places, and verifying the
> correctness of the call requires you to look up the declaration.

Maybe I don't understand but I don't think the code is meant
to provide means of easily changing types around.  It should
help understanding how the functions work.

Regards,

	Joey

-- 
Life is too short to run proprietary software.  -- Bdale Garbee

Please always Cc to me when replying to me on the lists.




Reply sent to Martin Schulze <joey@infodrom.org>:
You have taken responsibility. Full text and rfc822 format available.

Notification sent to Falk Hueffner <falk@debian.org>:
Bug acknowledged by developer. Full text and rfc822 format available.

Message #54 received at 348072-close@bugs.debian.org (full text, mbox):

From: Martin Schulze <joey@infodrom.org>
To: 348072-close@bugs.debian.org
Subject: Bug#348072: fixed in manpages 2.76-1
Date: Wed, 30 Jan 2008 21:47:02 +0000
Source: manpages
Source-Version: 2.76-1

We believe that the bug you reported is fixed in the latest version of
manpages, which is due to be installed in the Debian FTP archive:

manpages-dev_2.76-1_all.deb
  to pool/main/m/manpages/manpages-dev_2.76-1_all.deb
manpages_2.76-1.diff.gz
  to pool/main/m/manpages/manpages_2.76-1.diff.gz
manpages_2.76-1.dsc
  to pool/main/m/manpages/manpages_2.76-1.dsc
manpages_2.76-1_all.deb
  to pool/main/m/manpages/manpages_2.76-1_all.deb
manpages_2.76.orig.tar.gz
  to pool/main/m/manpages/manpages_2.76.orig.tar.gz



A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 348072@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Martin Schulze <joey@infodrom.org> (supplier of updated manpages package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.7
Date: Wed, 30 Jan 2008 22:20:57 +0100
Source: manpages
Binary: manpages manpages-dev
Architecture: source all
Version: 2.76-1
Distribution: unstable
Urgency: low
Maintainer: Martin Schulze <joey@debian.org>
Changed-By: Martin Schulze <joey@infodrom.org>
Description: 
 manpages   - Manual pages about using a GNU/Linux system
 manpages-dev - Manual pages about using GNU/Linux for development
Closes: 149554 236671 333871 348072 405694 435018 454007 459232 462636 462969
Changes: 
 manpages (2.76-1) unstable; urgency=low
 .
   * New upstream version
     . Add 2.6 details for /proc/sys/kernel/random/poolsize to random(4)
       (closes: Bug#459232)
   * Convert changelog and copyright from latin1 to utf8 (closes: Bug#454007)
   * Added override for lintian false positives
   * Properly quote ... and 'at the beginning of a line in stdarg(3) and
     bootparam(7) (closes: Bug#462636)
   * Adjusted proc(5) to not claim using bytes anymore (closes: Bug#462969)
   * Improved examples in bsearch(3) and qsort(3), thanks to Falk H├╝ffner
     (closes: Bug#348072)
   * Document overriding of certain passwd fields in nsswitch.conf(5),
     thanks to Vincent McIntyre (closes: Bug#333871)
   * Don't claim running a nameserver on localhost is normal in
     resolv.conf(5) (closes: Bug#149554)
   * Adjusted the reporting address in ioctl_list(2) (closes: Bug#236671)
   * Added a note on inet6 in resolv.conf(5) (closes: Bug#405694)
   * Fixed typo in vfork(2) (closes: Bug#435018)
Files: 
 536f3494b54940ddb2d47abc1cc89215 584 doc important manpages_2.76-1.dsc
 7e330eca6fb97c261b30b36684e54101 1277680 doc important manpages_2.76.orig.tar.gz
 ec52822323d50e91572168079a30d684 48149 doc important manpages_2.76-1.diff.gz
 3abfb14f19dd2e0f0774f0c72df2d701 549796 doc important manpages_2.76-1_all.deb
 6f4eacab0716b1b0a82200fb7d158f83 1362732 doc optional manpages-dev_2.76-1_all.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFHoOwCW5ql+IAeqTIRAhu3AJ9CRr7yXp5iXRv+HKz3ddd9J+1uzACfeUvg
UpzV9T81u1yLTUdq9pYlE0o=
=XC2T
-----END PGP SIGNATURE-----





Bug archived. Request was from Debbugs Internal Request <owner@bugs.debian.org> to internal_control@bugs.debian.org. (Thu, 28 Feb 2008 07:37:28 GMT) Full text and rfc822 format available.

Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Sun Apr 20 00:43:30 2014; Machine Name: beach.debian.org

Debian Bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.