All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: Collin Walling <walling@linux.ibm.com>
Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Halil Pasic" <pasic@linux.vnet.ibm.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Christian Borntraeger" <borntraeger@de.ibm.com>,
	qemu-s390x@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH for-2.13] pc-bios/s390-ccw: size_t should be unsigned
Date: Mon, 16 Apr 2018 09:31:05 +0200	[thread overview]
Message-ID: <20180416093105.180b2ce3.cohuck@redhat.com> (raw)
In-Reply-To: <c7dec4d0-2674-e51e-9698-571c7f02b6a7@linux.ibm.com>

On Fri, 13 Apr 2018 14:09:55 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> On 04/13/2018 02:06 PM, Philippe Mathieu-Daudé wrote:
> > On 04/13/2018 01:59 PM, Halil Pasic wrote:  
> >>>> On 04/13/2018 04:30 PM, Thomas Huth wrote:  
> >>>>> "size_t" should be an unsigned type - the signed counterpart is called
> >>>>> "ssize_t" in the C standard instead. Thus we should also use this  
> >>>> The first sentence sounds like ssize_t is too a type defined by some
> >>>> C standard. Is it or does ssize_t come form somewhere else?  
> >>> Arrr, seems like ssize_t is rather coming from POSIX than from the C
> >>> standard, thanks for the hint. I'll rephrase the first sentence to:
> >>>
> >>> "size_t" should be an unsigned type according to the C standard, and
> >>> most libc implementations provide a signed counterpart called "ssize_t".
> >>>
> >>> OK?
> >>>  
> >>
> >> This ssize_t seems to be an rather interesting type. For instance POSIX says
> >> """
> >> size_t
> >>     Used for sizes of objects.
> >> ssize_t
> >>     Used for a count of bytes or an error indication.
> >> """
> >> and
> >> """
> >> The type ssize_t shall be capable of storing values at least in the range [-1, {SSIZE_MAX}].
> >> """
> >>
> >> And it does not mandate SSIZE_MIN in limits (but of course mandates SSIZE_MAX.
> >>
> >> I don't like this 'counterpart' word here, because AFAIU these don't have to
> >> be counterparts in any sense. That is SSIZE_MAX << SIZE_MAX is possible for
> >> example. I'm not sure about the every positive has a negative thing, but
> >> that's not important here.
> >>
> >> The code in question kind of uses both signed and unsigned size for
> >> the same (the string). We even have a signed to unsigned comparison which
> >> could result in warnings. I still think the change is OK in practice, but
> >> maybe avoiding introducing ssize_t (until we really need it) is a better
> >> course of action. I think uitoa can be easily rewritten so it does not
> >> need the ssize_t.
> >>
> >> How about that?  
> > 
> > This seems clever indeed.
> >   
> 
> This whole issue stems from my misuse of size_t in the first place. If it makes
> things easier, let's just make num_idx of type "signed long".
> 
> After reading this discussion, I think it makes sense to drop ssize_t. No need
> to make it available for just one function unless there are strong claims to
> also use this type elsewhere in the pc-bios (I can't find any).
> 

+1 to just using signed long. Let's not make this needlessly complicated.

  reply	other threads:[~2018-04-16  7:31 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-13 14:30 [Qemu-devel] [PATCH for-2.13] pc-bios/s390-ccw: size_t should be unsigned Thomas Huth
2018-04-13 14:37 ` Philippe Mathieu-Daudé
2018-04-13 14:54 ` Collin Walling
2018-04-13 15:28 ` Halil Pasic
2018-04-13 15:40   ` Peter Maydell
2018-04-13 15:50   ` Thomas Huth
2018-04-13 16:59     ` Halil Pasic
2018-04-13 18:06       ` Philippe Mathieu-Daudé
2018-04-13 18:09         ` Collin Walling
2018-04-16  7:31           ` Cornelia Huck [this message]
2018-04-18 13:44       ` Eric Blake

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=20180416093105.180b2ce3.cohuck@redhat.com \
    --to=cohuck@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=f4bug@amsat.org \
    --cc=pasic@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=thuth@redhat.com \
    --cc=walling@linux.ibm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.