On Monday 11 February 2008 09:03, walter harms wrote: > ups, > now with lpr.h :)
char *buf = alloca(80);
Perhaps you want char buf[80].
int n;
if (buf == NULL)
exit(1);
Never happens. Usually alloca implemented so that it does not
do any checks for stack overflow.
buf[0] = 0;
do {
n = read(sockfd, buf, sizeof(buf));
read will read 4 or 8 bytes only. Also you need safe_read,
just in case...
if (n == -1)
break;
Should be if (n <= 0)... and then you won't need while(cond) -
while(1) will work too.
write(STDOUT_FILENO, buf, n);
// usleep(10000); /* LPD need some time to adjust */
usleep(100); /* LPD need some time to adjust */
} while (n == sizeof(buf));
Some time to adjust what? If lpd writes too slowly,
our reads will just block. Pure and simple. Am I missing anything?
if (n > 0 && buf[n] != '\n')
write(STDOUT_FILENO, "\n", sizeof("\n"));
Did you really meant to write two bytes here: '\n' + NUL?
static smallint CTRL_C;
static void get_ctrl_c(int sig)
{
CTRL_C = 1;
}
Use smallint instead.
/* queue info */
if (options.delay != 0)
signal(SIGINT, get_ctrl_c); /* ctrl-c terminates */
close(sockfd);
do {
sockfd = xconnect_stream(netprint.lsa);
fdprintf(sockfd, "%c%s\n", (options.flags & LPQ_SHORT) ? 3 : 4,
netprint.queue);
get_answer(sockfd);
sleep(options.delay);
close(sockfd);
} while (CTRL_C == 0 && options.delay != 0);
Why do you need to have Ctrl-C handler *at all*? Wouldn't just letting
Ctrl-C to kill you work too? Supposedly print server handles network
errors correctly - and if not, it's its own damn fault! :)
/*
lpr return 0 on success
else read the errormessage and exit
*/
static void get_response(int server_sock, const char *emsg)
{
int buf = 0;
safe_read(server_sock, &buf, 1);
if (buf != '\0') {
while (safe_read(server_sock, &buf, 1))
safe_write(STDOUT_FILENO, &buf, 1);
bb_perror_msg_and_die(emsg);
}
}
Too many bugs. Comment is wrong - it doesn't return anything.
Reading char into an int? If safe_read returns -1 (error),
it loops forever. If first read reads a byte, it will
always hit perror - so it is _always_ complaining? Why?
if (bb_copyfd_eof(local_fd, server_sock) < 0) {
bb_perror_msg_and_die("controlefile transfer failed");
}
bb_copyfd_eof internally does bb_perror_msg(bb_msg_read_error) or
bb_perror_msg(bb_msg_write_error)
on error, so you don't need another message. At least make it error, not perror.
if (argc < 1)
bb_perror_msg_and_die("argc < 1 ????");
Nice.
Both incarnations of handle_options() take argc and don't use it; and also
they return an int which is never used. I modified both to use
and return argv only.
atoi is too permissive, use xatoi_u, xatou etc.
make_controlfile() should create "controlfile" in memory, no point in saving it
to disk and delete after use.
if (lpr_options->flags & VERBOSE)
puts("verbose");
if (lpr_options->flags & USE_MAIL)
puts("use mail");
if (lpr_options->flags & USE_HEADER)
puts("use header");
Debug leftovers?
#define lpr_trivial_usage \
"-P [EMAIL PROTECTED]:port]] -U USERNAME -J TITLE -Vmh [filenames]"
#define lpr_full_usage \
"Options:" \
"\n -P lp service to connect to (else uses $PRINTER)"\
"\n -m Send mail to [EMAIL PROTECTED]" \
"\n -h Banner or header for this job" \
"\n -V Verbose" \
Where user shall specify [EMAIL PROTECTED]
-h description is confusing too.
Please add your (c) and licensing terms on top of each file.
See attached file. Some issues were filed, others are not.
Please grep for '//' comments to find problematic places.
Looking forward for next iteration of the patch.
--
vda
printutils.tar.bz2
Description: application/tbz
_______________________________________________ busybox mailing list [email protected] http://busybox.net/cgi-bin/mailman/listinfo/busybox
