On Wednesday 30 September 2009 15:03, Magnus Damm wrote:
> Hi everyone,
>
> Here comes a few patches that break out the progress meter code from
> wget and hooks it up to the tftp client. I'd like to see this feature
> picked up for inclusion in upstream busybox. That said, I realize that
> the patches may be far from perfect. So if you have any ideas on how
> to improve things please let me know and i'll do my best to rewrite
> and resubmit.
>
> Apply in the following order on top of
> 2f3f09c287f43dcad50b740793c2b467f166c058:
> 1: busybox-git-tftp-tsize-20090930.patch
I do not understand what this patch tries to achieve.
Please describe the reason for these changes.
For example:
-#if ENABLE_TFTPD
if (tsize) {
struct stat st;
/* add "tsize", <nul>, size, <nul> */
strcpy(cp, "tsize");
cp += sizeof("tsize");
+ st.st_size = 0;
fstat(local_fd, &st);
cp += snprintf(cp, 10, "%u", (int) st.st_size) + 1;
+ tsize = 0;
}
-#endif
Why do you remove "#if ENABLE_TFTPD"?
> 2: busybox-git-wget-break-out-progress-20090930.patch
+void bb_progress_init(bb_progress_t *p);
+void bb_progress_update(bb_progress_t *p, const char *curfile,
+ off_t beg_range, off_t transferred, off_t totalsize);
+void bb_progress_done(bb_progress_t *p);
Please add FAST_FUNC to these functions. See libbb.h how it is done
for other functions.
+void bb_progress_done(bb_progress_t *p)
+{
+ (void)p;
+ fputc('\n', stderr);
+}
This function seems to be useless. Users can just bb_putchar('\n')
themselves.
> 3: busybox-git-tftp-progress-20090930.patch
+/* SIGALRM logic nicked from the wget applet */
+static void progress_meter(int flag)
You duplicate the code this way.
It makes more sense to move SIGALRM code to libbb/progress.c
in previous patch, and just call it from wget.
> 4: busybox-git-progress-unknown-size-20090930.patch
Looks ok.
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
