LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sergei Organov <osv@javad.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Pekka Enberg" <penberg@cs.helsinki.fi>,
	"J.A. MagallÃÃÃón" <jamagallon@ono.com>,
	"Jan Engelhardt" <jengelh@linux01.gwdg.de>,
	"Jeff Garzik" <jeff@garzik.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Andrew Morton" <akpm@linux-foundation.org>
Subject: Re: somebody dropped a (warning) bomb
Date: Thu, 15 Feb 2007 21:53:04 +0300	[thread overview]
Message-ID: <87k5yjjlrj.fsf@javad.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0702150736360.20368@woody.linux-foundation.org> (Linus Torvalds's message of "Thu, 15 Feb 2007 07:57:05 -0800 (PST)")

Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Thu, 15 Feb 2007, Sergei Organov wrote:

[...Skip things I agree with...]

>> > But if you have 
>> >
>> > 	unsigned char *mystring;
>> >
>> > 	len = strlen(mystring);
>> >
>> > then please tell me how to fix that warning without making the code 
>> > *worse* from a type-safety standpoint? You CANNOT. You'd have to cast it 
>> > explicitly (or assing it through a "void *" variable), both of which 
>> > actually MAKE THE TYPE-CHECKING PROBLEM WORSE!
>> 
>> Because instead of trying to fix the code, you insist on hiding the
>> warning.
>
> No. I'm saying that I have *reasons* that I need my string to be unsigned. 
> That makes your first "fix" totally bogus:

Somehow I still suppose that most of those warnings could be fixed by
using "char*" instead. Yes, I believe you, you have reasons. But still I
only heard about isxxx() that appeared to be not a reason in practice
(in the context of the kernel tree). As you didn't tell about other
reasons, my expectation is that they are not in fact very common. I'm
curious what are actual reasons besides the fact that quite a lot of
code is apparently written this way in the kernel. The latter is indeed
a very sound reason for the warning to be sucks for kernel developers,
but it might be not a reason for it to be sucks in general.

>> There are at least two actual fixes:
>> 
>> 1. Do 'char *mystring' instead, as otherwise compiler can't figure out
>>    that you are going to use 'mystring' to point to zero-terminated
>>    array of characters.
>
> And the second fix is a fix, but it is, again, worse than the warning:
>
>> 2. Use "len = ustrlen(mystring)" if you indeed need something like C
>>    strings
>
> Sure, I can do that, but the upside is what?

1. You make it explicit, in a type-safe manner, that you are sure it's
   safe to call "strlen()" with "unsigned char*" argument. We all agree
   about it. Let's write it down in the program. And we can already add
   "strcmp()" to the list. And, say, ustrdup() and ustrchr() returning
   "unsigned char *", would be more type-safe either.

2. You make it more explicit that you indeed mean to use your own
   representation of strings that is different than what C idiomatically
   uses for strings, along with explicitly defined allowed set of
   operations on this representation.

The actual divergence of opinion seems to be that you are sure it's safe
to call any foo(char*) with "unsigned char*" argument, and I'm not, due
to the reasons described below. If you are right, then there is indeed
little or no point in doing this work.

> Getting rid of a warning that was bogus to begin with?

I agree that if the warning has no true positives, it sucks. The problem
is that somehow I doubt it has none. And the reasons for the doubt are:

1. C standard does explicitly say that "char" is different type from
   either "signed char" or "unsigned char". There should be some reason
   for that, otherwise they'd simply write that "char" is a synonym for
   one of "signed char" or "unsigned char", depending on implementation.

2. If "char" in fact matches one of the other two types for given
   architecture, it is definitely different from another one, so
   substituting "char" for the different one might be unsafe. As a
   consequence, for a portable program it might be unsafe to substitute
   "char" for any of signed or unsigned, as "char" may happen to
   differ from any of them.

While the doubts aren't resolved, I prefer to at least be careful with
things that I don't entirely understand, so I wish compiler give me a
warning about subtle semantic differences (if any).

-- Sergei.

  reply	other threads:[~2007-02-15 18:53 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-08 15:00 Jeff Garzik
2007-02-08 16:33 ` Linus Torvalds
2007-02-08 18:42   ` Jan Engelhardt
2007-02-08 19:53     ` Linus Torvalds
2007-02-08 21:10       ` Jan Engelhardt
2007-02-08 21:37         ` Linus Torvalds
2007-02-08 23:12           ` David Rientjes
2007-02-08 23:37             ` Linus Torvalds
2007-02-09  0:24               ` David Rientjes
2007-02-09  0:42                 ` Linus Torvalds
2007-02-09  0:59                   ` Linus Torvalds
2007-02-09  0:59                   ` David Rientjes
2007-02-09  1:11                     ` Linus Torvalds
2007-02-09  1:18                       ` David Rientjes
2007-02-09 15:38                         ` Linus Torvalds
2007-02-09  3:27                   ` D. Hazelton
2007-02-09 19:54                     ` Pete Zaitcev
2007-02-09 12:34                   ` Jan Engelhardt
2007-02-09 13:16                     ` linux-os (Dick Johnson)
2007-02-09 17:45                       ` Jan Engelhardt
2007-02-09 20:29                         ` linux-os (Dick Johnson)
2007-02-09 22:05                           ` Jan Engelhardt
2007-02-09 22:58                             ` Martin Mares
2007-02-12 18:50                             ` linux-os (Dick Johnson)
2007-02-13 15:14                     ` Dick Streefland
2007-02-08 21:13       ` J.A. Magallón
2007-02-08 21:42         ` Linus Torvalds
2007-02-08 22:03           ` Linus Torvalds
2007-02-08 22:19             ` Willy Tarreau
2007-02-09  0:03             ` J.A. Magallón
2007-02-09  0:22               ` Linus Torvalds
2007-02-09 12:38             ` Sergei Organov
2007-02-09 15:58               ` Linus Torvalds
2007-02-12 11:12                 ` Sergei Organov
2007-02-12 16:26                   ` Linus Torvalds
2007-02-13 18:06                     ` Sergei Organov
2007-02-13 18:26                       ` Pekka Enberg
2007-02-13 19:14                         ` Sergei Organov
2007-02-13 19:43                           ` Pekka Enberg
2007-02-13 20:29                             ` Sergei Organov
2007-02-13 21:31                               ` Jeff Garzik
2007-02-13 23:21                               ` Linus Torvalds
2007-02-15 13:20                                 ` Sergei Organov
2007-02-15 15:57                                   ` Linus Torvalds
2007-02-15 18:53                                     ` Sergei Organov [this message]
2007-02-15 19:02                                       ` Linus Torvalds
2007-02-15 20:23                                         ` me, not " Oleg Verych
2007-02-16  4:26                                         ` Rene Herman
2007-02-19 11:58                                           ` Sergei Organov
2007-02-19 13:58                                         ` Sergei Organov
2007-02-15 22:32                                     ` Lennart Sorensen
2007-02-13 19:25                       ` Linus Torvalds
2007-02-13 19:59                         ` Sergei Organov
2007-02-13 20:24                           ` Linus Torvalds
2007-02-15 15:15                             ` Sergei Organov
2007-02-13 21:13                         ` Rob Landley
2007-02-13 22:21                       ` Olivier Galibert
2007-02-14 12:52                         ` Sergei Organov
2007-02-15 20:06                         ` Sergei Organov
2007-02-09 15:10     ` Sergei Organov
2007-02-08 16:35 ` Kumar Gala
     [not found] <7Mj5f-3oz-21@gated-at.bofh.it>
     [not found] ` <7MktH-5EW-35@gated-at.bofh.it>
     [not found]   ` <7Mmvy-vj-17@gated-at.bofh.it>
     [not found]     ` <7MnBC-2fk-13@gated-at.bofh.it>
     [not found]       ` <7MoQx-4p8-11@gated-at.bofh.it>
     [not found]         ` <7MpjE-50z-7@gated-at.bofh.it>
     [not found]           ` <7MpCS-5Fe-9@gated-at.bofh.it>
     [not found]             ` <7MDd7-17w-1@gated-at.bofh.it>
     [not found]               ` <7MGkB-62k-31@gated-at.bofh.it>
     [not found]                 ` <7NHoe-2Mb-37@gated-at.bofh.it>
     [not found]                   ` <7NMe9-1ZN-7@gated-at.bofh.it>
     [not found]                     ` <7Oagl-6bO-1@gated-at.bofh.it>
     [not found]                       ` <7ObvW-89N-23@gated-at.bofh.it>
     [not found]                         ` <7Oc8t-NS-1@gated-at.bofh.it>
2007-02-15 20:08                           ` Bodo Eggert
2007-02-16 11:21                             ` Sergei Organov
2007-02-16 14:51                               ` Bodo Eggert
2007-02-19 11:56                                 ` Sergei Organov
2007-02-16 12:46                             ` Sergei Organov
2007-02-16 17:40                               ` Bodo Eggert
2007-02-19 12:17                                 ` Sergei Organov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k5yjjlrj.fsf@javad.com \
    --to=osv@javad.com \
    --cc=akpm@linux-foundation.org \
    --cc=jamagallon@ono.com \
    --cc=jeff@garzik.org \
    --cc=jengelh@linux01.gwdg.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penberg@cs.helsinki.fi \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: somebody dropped a (warning) bomb' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).