06

29

C言語プログラミング演習 解答1-1

2010.06.29(21:26)

まず最初の解答をもらいました。それに対してコメントをしました。


/**
* @file EncodeBase64.c
* @brief 文字列をBase64エンコードする。
* @par 2010.06.21 新規作成
*/

#include
#include "EncodeBase64.h"

/** 8bit/6bit文字用の単位 */ //★10
#define OCTET 8 //★20
#define SEXTET 6

/** 3個の8bit文字(合計24bit)が変換単位 */
#define OCTET_UNIT 3
/** 4個の6bit文字(合計24bit)が変換単位 */
#define SEXTET_UNIT 4

/** エンコード時の6bitマスク用 */
#define MASKING_SEXTET 0x00FC0000

/** 文字数埋めの文字 */
#define EQUAL '='

/** Base64文字の文字コード対応表 */
#define BASE64_TABLE_MAX 64
static char g_Base64Table[BASE64_TABLE_MAX] = {
'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J',
'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T',
'U', 'V', 'W', 'X', 'Y', 'Z', 'a', 'b', 'c', 'd',
'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n',
'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x',
'y', 'z', '0', '1', '2', '3', '4', '5', '6', '7',
'8', '9', '+', '/'};

/** プロトタイプ宣言 */
static void EncodeUnit(unsigned int Temp, int Unit, char *Encoded);

/**
* 文字列をBase64形式にエンコードする
* @brief エンコード前文字列を、変換単位ずつBase64エンコードする。
* 結果は文字列用配列の中身に直接格納する。
* @param エンコード前文字列、エンコード後文字列を格納する配列のポインタ
* @return なし。
* @note OutString配列のサイズは、InStringの文字数の約1.5倍以上の長さが必要
*/
void EncodeBase64(char *InString, char *OutString) //★30 //★40
{
/** 変換する文字を変換単位ごとに一時記憶 */
unsigned int unUnitTemp = 0;

/** エンコードする文字数 */
int nToEncode = 0; //★50

/** 文字数÷変換単位の余り */
int nRemainder = 0;

/** 場合によって追加する空白文字 */
char cAddition = 0;

int i = 0;
char *cpInHead = NULL;
char *cpOutHead = NULL;

/** 先頭アドレス記憶 */
cpInHead = InString;
cpOutHead = OutString;

/** 文字数を数え上げる */
while (*InString != '\0') {
nToEncode++;
InString++;
}

/** 変換単位ずつ変換 */
for (i = 0; i < nToEncode; i++) { //★60
unUnitTemp |= cpInHead[i];
if ((i + 1) % OCTET_UNIT == 0) {
EncodeUnit(unUnitTemp, SEXTET_UNIT, OutString); //★70
OutString += SEXTET_UNIT;
}
unUnitTemp <<= OCTET;
}

/** 余りの処理 */
nRemainder = nToEncode % OCTET_UNIT;
if (nRemainder) {
/** 最後尾に空白文字を追加して変換 */
InString = &cAddition;
unUnitTemp <<= OCTET;
EncodeUnit(unUnitTemp, (nRemainder + 1), OutString); //★80
}

OutString = cpOutHead;

return;
}

/**
* 指定された文字数だけ文字列をBase64形式にエンコードする
* @brief 結果は文字列用配列の中身に直接格納する。
* @param エンコード前文字列、エンコードする文字数、エンコード後文字列を格納する配列のポインタ
* @return なし。
*/
void EncodeUnit(unsigned int String, int n, char *Encoded)
{

unsigned int unMaskBit = 0x00000000;
int nShiftBit = 0;
int i = 0;

for (i = 0; i < SEXTET_UNIT; i++) {
Encoded[i] = EQUAL; //★90
}
for (i = 0; i < n; i++) {
unMaskBit = MASKING_SEXTET >> (SEXTET * i); //★100
nShiftBit = SEXTET * (SEXTET_UNIT - (i + 1));
Encoded[i] = g_Base64Table[ ((String & unMaskBit) >> nShiftBit) ]; //★110
}
return;
}


指摘内容
★10
自分のCソースの中だけで使っている定数なので、EncodeBase64.hではなくて、cの中で定義しているのはたいへんよい。EncodeBase64.h のほうには外部に公開されている定義だけをいれるようにしよう。

★20
定数定義とテーブルの定義が先頭にあるのは原則通りだが、実際に使う部分がずっと後のほうなので、少し読みにくくなってしまった。

★30
関数での処理が失敗する可能性がある場合は、呼び出し側で成功不成功がわかるようにしよう。

★40
入力データの長さ、出力バッファのサイズも渡してもらおう。
そして、出力バッファの長さが不足する場合は処理を打ち切ること。

★50
(signed) int を使うところと、size_t を使うべきところと区別しよう。

★60
もし、入力データの長さが0だったら?

★70
関数を呼び出す側でどういう結果になるかわかっていたとしても、
関数の側で、何バイト書き込んだかを求めて返してもらい、それを足したほうが
安全で読みやすく、仕様変更にも強い。

★80
先のEncodeUnit 呼び出しとの違いがわかりにくい。
おそらく、この関数には2つの責務があるのだろう。

★90
nがSEXTET_UNITの場合は、この処理がまったく無駄になる。

★100
少し何をやっているのか読み取りにくい。
わかりやすく書き換えるか、コメントをつけるか。

★110
定数の定義や、g_Base64Tableの定義がかなり遠くにあるので、
正しいかどうか読むのが難しい。

コメントの投稿

非公開コメント

プロフィール

島敏博

Shima Toshihiro 島敏博
信州アルプスハイランド在住。HaskellとElixirが好き。組み込みソフトウェアアーキテクト、C++プログラマ、山歩き、美術館巡り、和食食べ歩き、日本赤十字社救急法指導員、インデックス投資、クラシック音楽、SESSAME会員、状態マシン設計、モデル駆動開発、ソフトウェアプロダクトライン、Rubyist、実践ビジネス英語

■ ツイッター
http://twitter.com/saltheads
■ Facebook
http://www.facebook.com/saltheads
■ Qiita
http://qiita.com/saltheads

印刷する場合は、ブラウザの印刷メニューではなく、このページの上から3cmくらいの青いところにある、「印刷」を押してみてください。少しうまく印刷できます。まだ完全ではないのですが、これで勘弁してください。


カテゴリ
最新記事
月別アーカイブ
最新コメント
検索フォーム
リンク
sessame
RSSリンクの表示