From: "Ángel González" <ingenit@zoho.com>
To: hypsurus@mail.ru
Cc: "Stephan Müller" <fruktopus@gmail.com>, util-linux@vger.kernel.org
Subject: Re: New linux command "hexed"
Date: Sat, 09 May 2015 00:58:24 +0200 [thread overview]
Message-ID: <554D3F90.2000300@zoho.com> (raw)
In-Reply-To: <554CAC0F.5050604@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1711 bytes --]
On 08/05/15 14:29, Stephan Müller wrote:
> It seems you implemented a subset of the functionality of 'od' or 'xxd'
>from coreutils. I guess these are widely spread and there is no need for
>another converter (albeit the cooler name). If hexed provides some extra
>features, you can integrate those in the tools above.
xxd seems a perfect replacement for hexed :P
FWIW, I see a number of quirks in your tool, Hypsurus:
a) Options have to be provided *after* the string
Usually options always appear at the beginning (just after the program
name) and -when _POSIXLY_CORRECT is not set- the gnu optlib then allows
placing them at any position.
b) The program crashes if the string begins with a dash (str is NULL).
c) You program requires the string to be passed as parameter.
All programs like this have *filenames* as parameters. Consistency is
even more important for a program like this, that mimics the interfaces
of many other programs working the other way.
And receiving filenames makes much more sense. Do you really want to
type the binary contents as a program parameter? Does your OS even allow
that large? etc.
With a program that receives a filename, you can provide a string with
echo string | hexed
with a program that receives the string, you need a construct like:
hexed "$(</etc/passwd)"
and only if you use bash, the file is small enough to be passed as a
parameter and it doesn't contain any NUL (which defeats the point of an
hexer).
d) You have a number of signedness warnings, you are assgning a size_t
to an int and, generally, it would have been much easier for
hexed_hex_encode to work with unsigned char and hexed_hex_decode with
chars. See attached patch.
[-- Attachment #2: hexed.diff --]
[-- Type: text/x-patch, Size: 1188 bytes --]
--- hexed.c-original 2015-05-09 00:24:13.791707625 +0200
+++ hexed.c 2015-05-09 00:54:07.538368884 +0200
@@ -29,26 +29,29 @@
int addx;
int addCarray;
int hexd;
- unsigned char *str;
+ union {
+ char *str;
+ unsigned char *ustr;
+ };
} hexed_t;
void hexed_hex_encode(hexed_t *hexed) {
int index = 0;
- for( index = 0; hexed->str[index] != 0; index++ ) {
+ for( index = 0; hexed->ustr[index] != 0; index++ ) {
if ( hexed->addx )
- printf("\\x%x", hexed->str[index]);
+ printf("\\x%x", hexed->ustr[index]);
else if ( hexed->addCarray )
- printf("0x%x,", hexed->str[index]);
+ printf("0x%x,", hexed->ustr[index]);
else
- printf("%x", hexed->str[index]);
+ printf("%x", hexed->ustr[index]);
}
putchar(0x0a);
}
void hexed_hex_decode(hexed_t *hexed) {
- int index = 0;
+ size_t index = 0;
short byte = 0x00;
size_t len = strlen(hexed->str);
next prev parent reply other threads:[~2015-05-08 22:58 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-08 11:36 New linux command "hexed" Hypsurus
2015-05-08 12:29 ` Stephan Müller
2015-05-08 22:58 ` Ángel González [this message]
2015-05-11 7:51 ` Karel Zak
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=554D3F90.2000300@zoho.com \
--to=ingenit@zoho.com \
--cc=fruktopus@gmail.com \
--cc=hypsurus@mail.ru \
--cc=util-linux@vger.kernel.org \
/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.