Re: [PATCH] remove download_write_size variable

Vladimir Prus
2006-07-31T12:23:08Z

[-- Attachment #1: Type: text/plain, Size: 1999 bytes --]

On Wednesday 19 July 2006 00:42, Daniel Jacobowitz wrote:
> On Tue, Jul 18, 2006 at 04:48:44PM +0400, Vladimir Prus wrote:
> > This patch removes the 'download_write_size'. At the moment, is has
> > pretty low value, it's not clear why we need a separate option for just
> > one command, and the new qSupported packet allows to negotiate packet
> > size with the stub.
> >
> > So, I don't see any reason why this variable is needed, and given that
> > flash patches will seriously change symfile.c anyway, I suggest that we
> > "garbage-collect" this variable.
> >
> > Comments?
> >
> > - Volodya
> >
> > 2006-07-18 Vladimir Prus <vladimir@codesourcery.com>
> >
> > * symfile.c (download_write_size): Remove.
> > (show_download_write_size): Remove.
> > (load_section_callback): Don't use download_write_size.
> > (_initialize_symfile): Don't register download_write_size.
>
> I can find references to download-write-size on the Internet, e.g. in
> FAQs suggesting how to get better write performance. They all seem to
> push it as high as memory-write-packet-size, though, which suggests we
> do not need both limits.
>
> I think it's reasonable to remove it. However, you'll have to also
> remove it from the manual (it's a documented command).

Are you sure? Grep does not find any occurrence in docs, on current mainline.

> And I believe
> it should be added to NEWS as a removed feature.

Done.

> I'm going to let this patch sit a little while, in case anyone else can
> think of a reason to keep "set download-write-size".

The patch was sitting for quite some time already, OK to commit now? I'm
attaching it again for reference.

- Volodya

2006-07-31 Vladimir Prus <vladimir@codesourcery.com>

* symfile.c (download_write_size): Remove.
(show_download_write_size): Remove.
(load_section_callback): Don't use download_write_size.
(_initialize_symfile): Don't register download_write_size.
* NEWS: Mention 'download_write_size' removal.

[-- Attachment #2: download_write_size__gdb.diff --]
[-- Type: text/x-diff, Size: 2568 bytes --]

=== gdb/symfile.c
==================================================================
--- gdb/symfile.c (/mirrors/gdb) (revision 315)
+++ gdb/symfile.c (/patches/download_write_size/gdb) (revision 315)
@@ -1521,15 +1521,6 @@
we don't want to run a subprocess. On the other hand, I'm not sure how
performance compares. */

-static int download_write_size = 512;
-static void
-show_download_write_size (struct ui_file *file, int from_tty,
- struct cmd_list_element *c, const char *value)
-{
- fprintf_filtered (file, _("\
-The write size used when downloading a program is %s.\n"),
- value);
-}
static int validate_download = 0;

/* Callback service function for generic_load (bfd_map_over_sections). */
@@ -1570,11 +1561,6 @@
const char *sect_name = bfd_get_section_name (abfd, asec);
bfd_size_type sent;

- if (download_write_size > 0 && size > download_write_size)
- block_size = download_write_size;
- else
- block_size = size;
-
buffer = xmalloc (size);
old_chain = make_cleanup (xfree, buffer);

@@ -1591,8 +1577,6 @@
int len;
bfd_size_type this_transfer = size - sent;

- if (this_transfer >= block_size)
- this_transfer = block_size;
len = target_write_memory_partial (lma, buffer,
this_transfer, &err);
if (err)
@@ -3806,19 +3790,6 @@
add_info ("extensions", info_ext_lang_command,
_("All filename extensions associated with a source language."));

- add_setshow_integer_cmd ("download-write-size", class_obscure,
- &download_write_size, _("\
-Set the write size used when downloading a program."), _("\
-Show the write size used when downloading a program."), _("\
-Only used when downloading a program onto a remote\n\
-target. Specify zero, or a negative value, to disable\n\
-blocked writes. The actual size of each transfer is also\n\
-limited by the size of the target packet and the memory\n\
-cache."),
- NULL,
- show_download_write_size,
- &setlist, &showlist);
-
debug_file_directory = xstrdup (DEBUGDIR);
add_setshow_optional_filename_cmd ("debug-file-directory", class_support,
&debug_file_directory, _("\
=== gdb/NEWS
==================================================================
--- gdb/NEWS (/mirrors/gdb) (revision 315)
+++ gdb/NEWS (/patches/download_write_size/gdb) (revision 315)
@@ -18,6 +18,8 @@
Kernel Object Display, an embedded debugging feature which only worked with
an obsolete version of Cisco IOS.

+The 'download_write_size' variable.
+
* New remote packets