Commit graph

14 commits

Author SHA1 Message Date
Peter Jones
d5a32255de misc: Make grub_strtol() "end" pointers have safer const qualifiers
Currently the string functions grub_strtol(), grub_strtoul(), and
grub_strtoull() don't declare the "end" pointer in such a way as to
require the pointer itself or the character array to be immutable to the
implementation, nor does the C standard do so in its similar functions,
though it does require us not to change any of it.

The typical declarations of these functions follow this pattern:

long
strtol(const char * restrict nptr, char ** restrict endptr, int base);

Much of the reason for this is historic, and a discussion of that
follows below, after the explanation of this change.  (GRUB currently
does not include the "restrict" qualifiers, and we name the arguments a
bit differently.)

The implementation is semantically required to treat the character array
as immutable, but such accidental modifications aren't stopped by the
compiler, and the semantics for both the callers and the implementation
of these functions are sometimes also helped by adding that requirement.

This patch changes these declarations to follow this pattern instead:

long
strtol(const char * restrict nptr,
       const char ** const restrict endptr,
       int base);

This means that if any modification to these functions accidentally
introduces either an errant modification to the underlying character
array, or an accidental assignment to endptr rather than *endptr, the
compiler should generate an error.  (The two uses of "restrict" in this
case basically mean strtol() isn't allowed to modify the character array
by going through *endptr, and endptr isn't allowed to point inside the
array.)

It also means the typical use case changes to:

  char *s = ...;
  const char *end;
  long l;

  l = strtol(s, &end, 10);

Or even:

  const char *p = str;
  while (p && *p) {
	  long l = strtol(p, &p, 10);
	  ...
  }

This fixes 26 places where we discard our attempts at treating the data
safely by doing:

  const char *p = str;
  long l;

  l = strtol(p, (char **)&ptr, 10);

It also adds 5 places where we do:

  char *p = str;
  while (p && *p) {
	  long l = strtol(p, (const char ** const)&p, 10);
	  ...
	  /* more calls that need p not to be pointer-to-const */
  }

While moderately distasteful, this is a better problem to have.

With one minor exception, I have tested that all of this compiles
without relevant warnings or errors, and that /much/ of it behaves
correctly, with gcc 9 using 'gcc -W -Wall -Wextra'.  The one exception
is the changes in grub-core/osdep/aros/hostdisk.c , which I have no idea
how to build.

Because the C standard defined type-qualifiers in a way that can be
confusing, in the past there's been a slow but fairly regular stream of
churn within our patches, which add and remove the const qualifier in many
of the users of these functions.  This change should help avoid that in
the future, and in order to help ensure this, I've added an explanation
in misc.h so that when someone does get a compiler warning about a type
error, they have the fix at hand.

The reason we don't have "const" in these calls in the standard is
purely anachronistic: C78 (de facto) did not have type qualifiers in the
syntax, and the "const" type qualifier was added for C89 (I think; it
may have been later).  strtol() appears to date from 4.3BSD in 1986,
which means it could not be added to those functions in the standard
without breaking compatibility, which is usually avoided.

The syntax chosen for type qualifiers is what has led to the churn
regarding usage of const, and is especially confusing on string
functions due to the lack of a string type.  Quoting from C99, the
syntax is:

 declarator:
  pointer[opt] direct-declarator
 direct-declarator:
  identifier
  ( declarator )
  direct-declarator [ type-qualifier-list[opt] assignment-expression[opt] ]
  ...
  direct-declarator [ type-qualifier-list[opt] * ]
  ...
 pointer:
  * type-qualifier-list[opt]
  * type-qualifier-list[opt] pointer
 type-qualifier-list:
  type-qualifier
  type-qualifier-list type-qualifier
 ...
 type-qualifier:
  const
  restrict
  volatile

So the examples go like:

const char foo;			// immutable object
const char *foo;		// mutable pointer to object
char * const foo;		// immutable pointer to mutable object
const char * const foo;		// immutable pointer to immutable object
const char const * const foo; 	// XXX extra const keyword in the middle
const char * const * const foo; // immutable pointer to immutable
				//   pointer to immutable object
const char ** const foo;	// immutable pointer to mutable pointer
				//   to immutable object

Making const left-associative for * and right-associative for everything
else may not have been the best choice ever, but here we are, and the
inevitable result is people using trying to use const (as they should!),
putting it at the wrong place, fighting with the compiler for a bit, and
then either removing it or typecasting something in a bad way.  I won't
go into describing restrict, but its syntax has exactly the same issue
as with const.

Anyway, the last example above actually represents the *behavior* that's
required of strtol()-like functions, so that's our choice for the "end"
pointer.

Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-02-28 12:41:29 +01:00
Gustavo Luiz Duarte
fc085f7f18 net: Fix crash on http
Don't free file->data on receiving FIN flag since it is used all over
without checking. http_close() will be called later to free that memory.

Fixes bug: https://bugzilla.redhat.com/show_bug.cgi?id=860834

Signed-off-by: Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2019-09-23 13:16:48 +02:00
Stefan Fritsch
13f7ead3a1 http: reset EOF indication in http_seek
Otherwise next read will stop polling too early due to stale EOF
indicator, returning incomplete data to caller.
2016-04-30 09:13:34 +03:00
Michael Chang
abf9beb7d6 http: fix superfluous null line in range request header
At least the apache sever is very unhappy with that extra null line and will
take more than ten seconds in responding to each range request, which slows
down a lot the entire http file transfer process or even time out.
2016-04-09 07:08:13 +03:00
Vladimir 'phcoder' Serbinenko
92c8f58d97 * grub-core/net/http.c: Fix bad free. 2013-04-12 20:17:53 +02:00
Vladimir 'phcoder' Serbinenko
982f1909cf Implement flow control for http.
* grub-core/net/http.c (parse_line): Handle response 206.
	(http_receive): Stall if too many packets are in the queue.
	(http_establish): Fix range header.
	(http_seek): Fix double free.
	(http_close): Likewise.
	(http_packets_pulled): New function.
	(grub_http_protocol): Set http_seek
	* grub-core/net/tcp.c (grub_net_tcp_socket): New field `i_stall'.
	(ack_real): Set window depending on i_stall.
	(grub_net_send_tcp_packet): Likewise.
	(grub_net_tcp_stall): New function.
	(grub_net_tcp_unstall): Likewise.
	* include/grub/net/tcp.h (grub_net_tcp_stall): New proto.
	(grub_net_tcp_unstall): Likewise.
2012-06-22 22:09:31 +02:00
Vladimir 'phcoder' Serbinenko
b27069e06d Implement flow control for tftp.
* grub-core/net/net.c (receive_packets): Decrease the stop to 10
	packets but stop only if stop condition is satisfied.
	(grub_net_fs_read_real): Call packets_pulled after real read. Use
	`stall' instead of `eof' as stop condition.
	* grub-core/net/http.c (parse_line): Set `stall' on EOF.
	(http_err): Likewise.
	* grub-core/net/tftp.c (ack): Replace the first argument with data
	instead of socket.
	(tftp_receive): Stall if too many packets are in wait queue.
	(tftp_packets_pulled): New function.
	(grub_tftp_protocol): Set packets_pulled.
	* include/grub/net.h (grub_net_packets): New field count.
	(grub_net_put_packet): Increment count.
	(grub_net_remove_packet): Likewise.
	(grub_net_app_protocol): New field `packets_pulled'.
	(grub_net): New field `stall'.
2012-06-22 14:17:46 +02:00
Vladimir 'phcoder' Serbinenko
96f7e60eae Stop polling as soon as we have the packet we were waiting for.
* include/grub/net.h (grub_net_poll_cards): New argument stop_condition.
	All users updated.
	* grub-core/net/arp.c (have_pending): New var.
	(pending_req): Likewise.
	(grub_net_arp_send_request): Fill pending_req and use have_pending as
	stop indicator.
	(grub_net_arp_receive): Set have_pending.
	* grub-core/net/dns.c (recv_data): New field stop.
	(recv_hook): Set stop.
	(grub_net_dns_lookup): Init stop and use as stop condition.
	* grub-core/net/http.c (http_establish): Use headers_recv as stop
	condition.
	* grub-core/net/net.c (grub_net_poll_cards): New argument
	stop_condition. Stop when it goes true.
	* grub-core/net/tcp.c (grub_net_tcp_open): Use `established' as stop
	indicator.
	* grub-core/net/tftp.c (tftp_open): Use `have_oack' as stop indicator.
2012-06-09 11:06:55 +02:00
Vladimir 'phcoder' Serbinenko
ef292a8775 * grub-core/net/http.c: Add TRANSLATORS comments.
* grub-core/normal/cmdline.c: Likewise.
	* grub-core/normal/misc.c: Likewise.
	* grub-core/partmap/msdos.c: Likewise.
	* grub-core/parttool/msdospart.c: Likewise.
	* grub-core/script/execute.c: Likewise.
	* grub-core/script/main.c: Likewise.
	* grub-core/term/terminfo.c: Likewise.
	* grub-core/video/bitmap.c: Likewise.
	* util/grub-install.in: Likewise.
	* util/grub-mkimage.c: Likewise.
	* util/grub-mklayout.c: Likewise.
	* util/grub-setup.c: Likewise.
2012-03-05 16:42:26 +01:00
Vladimir 'phcoder' Serbinenko
d61386e21d Improve string. Gettextize. 2012-02-12 15:25:25 +01:00
Vladimir 'phcoder' Serbinenko
9c4b5c13e6 Improve gettext support. Stylistic fixes and error handling fixes while
on it.
2012-02-08 19:26:01 +01:00
Vladimir 'phcoder' Serbinenko
bd40efbf0b several net bugfixes and improvements and fix some memory leaks 2011-10-13 18:31:53 +02:00
Vladimir 'phcoder' Serbinenko
4f71e077b9 HTTP seek support. Various bugfixes. 2011-10-05 17:39:13 +02:00
Vladimir 'phcoder' Serbinenko
80e722366d First attempt at http 2011-07-23 03:49:02 +02:00